PERFORMANCE: Intern Multiple String Constants #4754
Closed
+174
−110
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.
JRuby
intern
s mostString
s that come up in Ruby code just fine inorg.jruby.ir.persistence.IRReaderStream#decodeString
:, but a few duplicates from more dynamic contexts are not caught. In the red-black benchmark, this adds up to ~200kb of wasted memory (memory wise this is irrelevant obviously, but duplicates make call site cache lookups slower and waste CPU cache => fixing all these spots resulted in a visible ~2% speedup in the red-black benchmark for me over very large sample sizes 2k+ runs).
So I:
AST
ish code pathsintern
byintern
ing in constructors wherever possible instead of further upstreamintern
calls made it into the hot path by checking that the method isn't called in the hot path (checked red-black benchmark and Logstash after warm up andintern
was never called in either => I have a bit of confidence)Also:
public synchronized void defineAliases(List<String> aliases, String oldName)
since it was one of the cases that looked like they would create duplicateString
constants but then realized that it's never used (checked for dynamic invocations too)public static JavaCallable getMatchingCallable(Ruby runtime, Class<?> javaClass, String methodName, Class<?>[] argumentTypes)
since this method was in a path affected by the addedintern
calls and only usedprivately
=> I didn't want to add redundant calls tointern
to it, when I can be sure that its only callerintern
sString
s just fine already