rescanMac: Avoid overallocating/overreading by millions of bytes #22928
+2
−4
Conversation
This file contains 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
jacobly0
reviewed
Feb 18, 2025
squeek502
force-pushed
the
millions-of-bytes
branch
from
813463e to
e7caeb3
Compare
readAtLeast is greedy and will read the entire length of the buffer if it can. However, reading past the end of the cert in this case is useless, so reading the full length of the buffer just puts an increasingly large (due to the growth algorithm of ArrayList) collection of wasted bytes after each cert in cb.bytes. In practical terms, this ends up saving potentially millions of bytes of wasted reads/allocations. In my testing, after reading the keychain files on my machine, cb.bytes ends up with these capacities: - Before: cb.bytes.capacity = 32720747 - After: cb.bytes.capacity = 251937 That's a decrease of 99.2% Additionally, swaps to readNoEof since it should be an error to hit EOF without reading the full cert size.
squeek502
force-pushed
the
millions-of-bytes
branch
from
e7caeb3 to
c6bf46f
Compare
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.
Something random I noticed while investigating #22870
readAtLeast is greedy and will read the entire length of the buffer if it can. However, reading past the end of the cert in this case is useless, so reading the full length of the buffer just puts an increasingly large (due to the growth algorithm of ArrayList) collection of wasted bytes after each cert in cb.bytes.
In practical terms, this ends up saving potentially millions of bytes of wasted reads/allocations. In my testing, after reading the keychain files on my machine, cb.bytes ends up with these capacities:
That's a decrease of 99.2%
Additionally, swaps to readNoEof since it should be an error to hit EOF without reading the full cert size.