Prevent memory allocation on symbolized keys #1
+7
−1
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.
A lot of memory is allocated in
jbuilder's_keymethod. Doing something as simple asjson.set! :key, 'whatever'allocates a string for the key. Since this runs for every json property, the memory allocation adds up. This can result in more frequent and longer garbage collection cycles, which adds latency.For some context, waaaay back in 20212 there was a PR filed titled "Substantial performance improvements". One of the big pieces here were optimizations to key formatting. The solution introduced was a caching mechanism that would only format a key (often a symbol) to a string a single time. The default formatter that would get applied would simply be a
to_s, so a given key like:foowould become'foo'and that would be cached.I suppose this was deemed overkill for simple key value pairs, and thus another PR was filed to make the key formatter optional, and instead just return the key as a string if no key formatter was explicitly provided. On the surface this makes sense, but this is where the problem was introduced: this causes a new string allocation for each key. That PR provides some benchmarks that look promising, but the benchmark didn't account for the additional memory allocations (which can offset any performance gains due to GC cycles) and didn't benchmark against something that would actually yield cache hits - the code under bench ran a single time, and even if it didn't,
samplewould randomly select 1 of 100 keys which are unlikely to be cached.Regardless, there is an easy way to keep the key formatter optional without introducing extra memory overhead by using
Symbol#nameinstead ofSymbol#to_s. The former results in an interned string so that same string object is used, preventing extra memory allocation.While keys are very likely to be
Symbols since that is whatmethod_missingprovides when doing something likejson.name :foo, to maintain backwards compatibility the call to.namewill only be attempted if the key is in fact aString; it's possible there are things in the wild that don't use symbols as keys (ex:json.set! "foo", :bar).Comparing the
_keymethod directly, you not only see the extra allocation saved but also a performance improvement.This performance gain is also noticed when using the
jbuilderDSL as well. For example