Skip to content

Track memory usage over time#38

Merged
ctz merged 4 commits into
mainfrom
jbp-memory
Nov 13, 2025
Merged

Track memory usage over time#38
ctz merged 4 commits into
mainfrom
jbp-memory

Conversation

@ctz

@ctz ctz commented Nov 7, 2025

Copy link
Copy Markdown
Member

This passes tests. Manually tested with rustls/rustls#2749

@ctz ctz force-pushed the jbp-memory branch 2 times, most recently from 1920a7f to b4ec164 Compare November 7, 2025 17:27
@ctz ctz marked this pull request as ready for review November 10, 2025 09:10
@ctz ctz requested a review from djc November 10, 2025 09:10

@djc djc left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks okay to me? I'm very unfamiliar with this codebase, not sure if there's anything in particular you want me to look for or where I go to look at the results.

@ctz

ctz commented Nov 11, 2025

Copy link
Copy Markdown
Member Author

Yeah, I'm similarly unfamiliar. The version at the head of this PR is the one deployed, so the output is being included in PRs right now.

eg, rustls/rustls#2756 (comment) which says, for handshake_tickets_1.2_no_crypto_client:

⚠️
∑ +960B +0a
🔝 +768B +0a (0.73%)

The first line means: cumulatively over the whole test, there were 960 bytes more heap used, but no additional allocations. That means some allocated structure got bigger. It is not possible to say whether the structure got bigger by 960 bytes, or by (say) 96 bytes but was allocated ten times over the test.

The second line says a similar thing, but is a snapshot at the peak heap memory usage.

@djc

djc commented Nov 11, 2025

Copy link
Copy Markdown
Member

Uhh, not sure I'm seeing that...

Screenshot 2025-11-11 at 15 39 00

@ctz

ctz commented Nov 11, 2025

Copy link
Copy Markdown
Member Author

Hm, odd. Looking into that.

@djc

djc commented Nov 11, 2025

Copy link
Copy Markdown
Member

Seeing it now. Perhaps it gets reset when I push?

@djc

djc commented Nov 11, 2025

Copy link
Copy Markdown
Member

The first line means: cumulatively over the whole test, there were 960 bytes more heap used, but no additional allocations. That means some allocated structure got bigger. It is not possible to say whether the structure got bigger by 960 bytes, or by (say) 96 bytes but was allocated ten times over the test.

The second line says a similar thing, but is a snapshot at the peak heap memory usage.

Okay, good stuff!

This didn't work for memory results, because the "headline" figure
only is persisted in the database (to match the assumption that
any measure is a f64, whereas memory measurements are more structured
than that.)

This means force-pushes to PRs will re-run and re-measure the benchmarks,
naturally that is slower than _not_ doing that, but also takes about two
minutes currently.
@ctz

ctz commented Nov 12, 2025

Copy link
Copy Markdown
Member Author

Seeing it now. Perhaps it gets reset when I push?

Yep. I think I have fixed this, and deployed 294f856

@ctz ctz merged commit bb5220c into main Nov 13, 2025
9 checks passed
@ctz ctz deleted the jbp-memory branch November 13, 2025 09:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants