[classlib][jit][opt][performance] Optimize heap allocations in String(String, int) constructor for JIT

• Key: HARMONY-6056
• Type: Improvement
• Status: Resolved
• Resolution: Fixed
• Priority: Major
• Assignee: Tim Ellison
• Reporter: Kevin Zhou
• Component/s: Classlib
• Affects Version/s: 5.0M8
• Fix Version/s: 5.0M9

Description

This patch optimizes heap allocations in String(String, int) constructor for JIT performance improvement.

Comments

[Kevin Zhou]

Would you please help to try it?

[Nathan Beyer]

Can you provide some context and background to this patch? Thanks

[Kevin Zhou]

The private String(String, int) constructor is too heavy in its current implementation as it uses String.valueOf(int), which in turn uses Integer.toString(int), which contains two heap allocations inside. Apparently, if we use division to convert an integer to string can be much faster.
In addition, the changes on String.indexOf method can improve SPECjbb2005 score.

[Kevin Zhou]

I hope that this would be valid for you

[Aleksey Shipilev]

Kevin, why not to patch Integer.toString(int) instead?

[Kevin Zhou]

Integer.toString(int) contains two heap allocations inside which may be much slower than dividing and converting an integer to string.

[Tim Ellison]

Aleksey: are you ok with this patch?

[Aleksey Shipilev]

Had a annual memory reset during NY :)

I agree with Kevin's rationale. Are you sure that the performance boost is about heap allocation? The code in the patch and Integer.toString(int) look different.

On the other hand, if the added code in String(String, int) is inlined Integer.toString(int), this is a code duplication. My first thought was: to avoid that, can we just make package-private Integer.toString(char[], int, int), where we can put the target array, offset and the value? Then in String(String, int) we may just call that method. And finally, we can reduce the code duplication by calling the same method from Integer.toString() :)

It would also testify the performance boost is related to heap allocation, not the changed conversion implementation.
If the boost is connected with specialized conversion for radix 10, then it's rather better to specialize it at Integer, rather than at String :)

Does that makes sense?

[Aleksey Shipilev]

Just flashed into my mind.
Kevin, what javac/JVM you use to make the performance runs? Is it J9 or DRLVM? Are there any steps to reproduce the result?

AFAIK, DRLVM does not substitute the "String s2 = (String)s1 + (int)v1;" to "new String(s1, v1)", is it J9 who does this thing? If so the another benefit of generalizing your solution in Integer.toString(), because DRLVM can then tap on your probably-optimized radix-10 conversion when concatenating strings via StringBuilder.

[Aleksey Shipilev]

StringBench.jar

Simple benchmark to evaluate "(String)base + (int)add" operation performance. I presume that the attached patch should increase the score on this benchmark.

On Harmony-M7 at P-D 2.8 Ghz / PC2-5300 / Gentoo Linux x86:

$ harmony-M7/bin/ava -Xms1024M -Xmx1024M -server -jar StringBench.jar
(String)base + (int)add:
-------------------------------------------
 base length (vars with rows): 0..2..10
  add length (vars with cols): 0..2..10

 loop duration = 100 msecs
 target variance = 0.05

ops/msec, the more the better:
4369, 2842, 1976, 1563, 1226,
4462, 2829, 1966, 1555, 1225,
4430, 2836, 1956, 1563, 1223,
4394, 2831, 1956, 1558, 1217,
4357, 2821, 1954, 1551, 1217,

[Tim Ellison]

Do we agree to close this now?

[Kevin Zhou]

Merge with HARMONY-6068. This can be closed now.

[Aleksey Shipilev]

Just wondering: is the boost from HARMONY-6068 equals to this patch, while running on J9?
Where did it come from finally: reduced heap allocation or optimized conversion?