test: add memory profiler test #5329
Conversation
| /// Because the s2n-tls rust bindings set the s2n-tls memory callbacks to use the | ||
| /// rust allocator, this does give a good picture of s2n-tls allocations. However | ||
| /// this will _not_ report the allocations done by the libcrypto. |
There was a problem hiding this comment.
Is this why you're not removing the existing memory usage test?
| /// 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
| pair.io.client_tx_stream.borrow().capacity() + pair.io.server_tx_stream.borrow().capacity() | ||
| } | ||
|
|
||
| fn fuzzy_equals(actual: usize, expected: usize) -> bool { |
There was a problem hiding this comment.
Why is fuzzy_equals needed? Is this test nondeterministic, or do you just not want to update it all the time?
Description of changes:
Our current memory usage unit test is incredibly fuzzy. We can improve the accuracy by using an actual memory profiler to get the exact amount allocated.
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.