Remove unnecessary call to trimStringObjectIfNeeded from tryObjectEnc…#11509
Closed
uriyage wants to merge 1 commit into
Closed
Remove unnecessary call to trimStringObjectIfNeeded from tryObjectEnc…#11509uriyage wants to merge 1 commit into
uriyage wants to merge 1 commit into
Conversation
oranagra
reviewed
Nov 15, 2022
Member
There was a problem hiding this comment.
i don't like this change.
what it does is trim the excess space every time we put a large string into the argv array, even if that string isn't gonna end up retained, i.e. stored into the database.
the approach of the existing code, it to trim it only when we decide to keep it.
i'd rather change the code in trimStringObjectIfNeeded to be smarter and avoid excessive work.
i.e. it tries to get rid of 10% excess, but in fact the the allocator usually has up to 30% internal fragmentation, so this really became out of context when i made #7875.
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.
Call
trimStringObjectIfNeededonly when using thequerybufas the object's ptr.Currently, we call
trimStringObjectIfNeededfromtryObjectEncodingwhich is called for all set-commands.However, the only place where we might allocate a larger than necessary string object is when we use the
querybufitself as the object's value.trimStringObjectIfNeededmight be expensive as it tries to remove the internal fragmentation (a result of #7875 change).