Conversation
| /// This isn't totally accurate because it doesn't account for any indirection that | ||
| /// may be present. |
There was a problem hiding this comment.
I'm not sure I understand why this wouldn't be accurate. capacity() should be accurate, right? Is this saying that server_tx_stream or client_tx_stream might be pointing to some other LocalDataBuffer, so it may not be returning the size of the actual TestPairIO?
There was a problem hiding this comment.
Nope, it's saying that there may be sneaky other allocations hidden :). Specifically, this method fails to account for 80 heap allocated bytes because the RefCell and it's contents are also heap allocated
* add assertion on max memory allocated
|
This PR has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
* forgot to remove the other network tests * rustfmt
| println!("static memory: {static_memory}"); | ||
| println!("config: {config_init}"); | ||
| println!("connection_init: {connection_init}"); | ||
| println!("handshake in progress: {handshake_in_progress}"); | ||
| println!("handshake complete: {handshake_complete}"); | ||
| println!("application data: {application_data}"); |
There was a problem hiding this comment.
Since this test is deterministic, rather than have to update this test every so often when a threshold is breached, I wonder if we could alternatively just have this program write these values to a file as a snapshot test? Then there could be a test that just diffs this file and makes you run this program and commit the new result whenever there's a change.
It might be cool to see the change in memory in each PR diff, even if the difference in memory usage is small. This would also avoid needing to update the thresholds manually.
There was a problem hiding this comment.
I'm open to this 🤷 . @lrstewart thoughts?
Co-authored-by: Sam Clark <3758302+goatgoose@users.noreply.github.com>
Co-authored-by: Sam Clark <3758302+goatgoose@users.noreply.github.com>
Description of changes:
Goal
Accurately assert on the memory usage of s2n-tls.
Why
Our current memory usage unit test is very fuzzy. We can get much more accurate numbers of using a memory profiler.
This will also allow us to emit metrics and track memory usage over time, the same way we do with our benchmarking numbers.
How
We use the dhat-rs memory profiler (which is totally separate from the valgrind project of the same name). This requires overriding AWS-LC memory callback to use the rust system allocator so that we can also have visibility into memory usage at the libcrypto layer.
This PR gates the memory test to only run on a single CI job, because it is sensitive (as designed) to changes in platform/features, because that changes the size of structs/other items.
Testing:
Ran locally. Also numbers relatively closely match our existing memory tests.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.