feat: Add memory usage test framework#272
Draft
tmathern wants to merge 13 commits into
Draft
Conversation
Clarify explanations regarding memory leaks and temporary allocations in the README.
Updated documentation to reflect SDK operations instead of just read and sign operations.
* fix: Updated code * fix: Updated code
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What is this?
This tool uses memray to profile memory use and create HTML reports and graph showing memory use. It is configurable (runs count, default to 100 and environment through Docker image for reproduceability).
How to read the graphs?
It helps to separate the two lines on the graph, because they answer different questions and only one of them is about leaks.
Heap size (the lower line, orange in the reports): memory currently allocated and not yet freed. This is the line that detects leaks.
Resident set size / RSS (the upper line, blue in reports, more wavy): total memory the process holds from the operating system, including memory the allocator has already freed internally but not yet returned to the OS.
A memory leak is memory that is allocated and never freed, so it accumulates in proportion to work done. The direct measurement of that is the heap line, and across all multiple iterations it is flat (baseline was generated using 1000 iterations on each scenario). It rises and falls within each iteration (allocate while signing, free at the end) and returns to the same baseline every single time. It does not trend upward.
The iteration count is what makes this interesting. If one sign operation leaked even 1 KB, then after 1000 iterations the heap floor would have risen, which would be visible. The floor (bottom line/orange line) doesn't move. A leak cannot be flat: growth with work is the thing that makes it a leak. Therefore, seeing growth is indicative of a leak; seing a flat line means no leak.
The upper RSS line does drift upward in some scenarios, and that's the part that looks concerning initally. When memory is freed, the allocator (glibc malloc/pymalloc) does not always hand those pages back to the OS immediately (on purpose). It keeps them in its own pool to serve future allocations, and fragmentation leaves the resident footprint sitting at a high-water mark above the live total. You can see this directly on the graphs: the RSS line has downward steps where the allocator does return pages, which is only possible because that memory was free rather than leaked. And since these runs were captured with native tracking enabled, the heap line already includes the native allocations, not just Python, so a native-side leak would also show up on that flat line (line would trend upwards).
For anyone who wants to verify it independently rather than take the graph's word for it, the check is built in and reproducible: run the harness at increasing iteration counts and watch leaked_bytes. A real leak scales with iterations, so if there is a leak it will be made visible by the upward trend of the line.
Checklist
TO DOitems (or similar) have been entered as GitHub issues and the link to that issue has been included in a comment.