Optimize _key
to prevent string allocation when formatting Symbol
s
#593
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_key
method. 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:foo
would 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,
sample
would 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#name
instead 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
Symbol
s since that is whatmethod_missing
provides when doing something likejson.name :foo
, to maintain backwards compatibility the call to.name
will 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
_key
method directly, you not only see the extra allocation saved but also a performance improvement.This performance gain is also noticed when using the
jbuilder
DSL as well. For example