Merged
Conversation
bazsi
force-pushed
the
logmsg-memory-allocation
branch
7 times, most recently
from
64b0c97 to
8cda489
Compare
bazsi
force-pushed
the
logmsg-memory-allocation
branch
2 times, most recently
from
c3362cc to
cd4ef81
Compare
Member
Author
|
I decided to split off the controversial part of this PR into a separate branch (which shrinks the message payload when adding to the queue), so this one should be easier/simpler to review/merge. |
OverOrion
requested review from
OverOrion and
sodomelle
and removed request for
OverOrion
bazsi
force-pushed
the
logmsg-memory-allocation
branch
from
cd4ef81 to
9cf36d1
Compare
Signed-off-by: Balazs Scheidler <[email protected]>
This functionality is going to be refactored into a more elaborate algorithm to resize an NVTable, assuming a new storage needed. To be used both from nv_table_clone() and nv_table_resize(), in a next patch. Signed-off-by: Balazs Scheidler <[email protected]>
Whenever NVTable needs to grow (either during clone or realloc), the new nv_table_get_next_size() function calculates the right new size, while ensuring some "additional_space" becomes available in the new clone. Previously, we always erred on the side of increasing the allocation size, so that any expected new changes would not need another bump of the size. This algorithm however, can waste a lot of memory. The new algorithm is: * increase at least 256 bytes, * only increase the size if the current allocation does not fit * use 4k aligned sizes for large NVTables (>4kB) or 1k for small ones We might have slightly more reallocs this way, but the total allocation of a LogMessage should be a lot less. Signed-off-by: Balazs Scheidler <[email protected]>
…e_realloc() Signed-off-by: Balazs Scheidler <[email protected]>
…() function We do not support cases where these would point to different instances, so reduce redundancy by using a single in/out pointer. Signed-off-by: Balazs Scheidler <[email protected]>
In case our "payload" would be larger than 2kB, do not piggy back the payload at the end of the LogMessage. Signed-off-by: Balazs Scheidler <[email protected]>
Signed-off-by: Balazs Scheidler <[email protected]>
bazsi
force-pushed
the
logmsg-memory-allocation
branch
from
9cf36d1 to
da206c0
Compare
sodomelle
approved these changes
Jan 30, 2026
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
2 participants
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.
This PR improves the memory used by LogMessage in a number of ways, as 1M messages can consume surprisingly large amount of RAM today.
Storing 1M messages in a memory queue with the patch:
Without the patch (but with my memory accounting fixes), same 1M messages:
Interestingly, the performance even improved with the patched version, even though it is doing an extra realloc. (29.592s vs 28.377s), but I'd need to rerun my tests.
Patched metrics, 6483 bytes per message on average, containing both a JSON text (2kB) and a flattened version of the JSON
@orymate