RFR: 8061254: SPECjvm2008-XML performance regressions in 9-b33 (original) (raw)

Vitaly Davidovich vitalyd at gmail.com
Wed May 13 12:53:02 UTC 2015

It's interesting that this version performs as good or better than value.length > 0 check. Intuitively, value.length is going to be used anyway (if h == 0) in the loop and so there should be no penalty of loading and branching on it (given the change here has a branch anyway) and compiler can keep that value in a register for the actual loop.

Looking at the generated asm for current String.hashCode(), it doesn't look like compiler is actually using the user-written check to skip its own check (see the two tests at the bottom of this snippet):

0x00007f7e312a0d8c: mov %rsi,%rbx 0x00007f7e312a0d8f: mov 0x10(%rsi),%eax ;*getfield hash ; - java.lang.String::hashCode at 1 (line 1454)

0x00007f7e312a0d92: test %eax,%eax 0x00007f7e312a0d94: jne 0x00007f7e312a0e85 ;*ifne ; - java.lang.String::hashCode at 6 (line 1455)

0x00007f7e312a0d9a: mov 0xc(%rsi),%ebp ;*getfield value ; - java.lang.String::hashCode at 10 (line 1455)

0x00007f7e312a0d9d: mov 0xc(%r12,%rbp,8),%r10d ;*arraylength ; - java.lang.String::hashCode at 13 (line 1455) ; implicit exception: dispatches to 0x00007f7e312a0ea9 0x00007f7e312a0da2: test %r10d,%r10d 0x00007f7e312a0da5: jle 0x00007f7e312a0e91 ;*ifle ; - java.lang.String::hashCode at 14 (line 1455)

0x00007f7e312a0dab: test %r10d,%r10d 0x00007f7e312a0dae: jbe 0x00007f7e312a0e95

It does however continue using r10.

On Wed, May 13, 2015 at 8:36 AM, Sergey Bylokhov <Sergey.Bylokhov at oracle.com

wrote:

Just curious, what is the difference between this fix and an old version of the code:

http://cr.openjdk.java.net/~shade/8058643/webrev.01/src/java.base/share/classes/java/lang/String.java.sdiff.html I meant: "if (h == 0 && value.length > 0) {}" vs "if (h != 0) {hash = h;}"

On 13.05.15 14:51, Claes Redestad wrote: Hi,

9-b33 introduced a sustained regression in SPECjvm2008 xml.transform on a number of our test setups. JDK-8058643 removed the check on value.length > 0, which means repeated calls to "".hashCode() now do a store of the calculated value (0) to the hash field. This has potential to cause excessive cache coherence traffic in xml.transform[1] Extracting the code that showed up in profiles to a micro[2] and running this in multiple threads is up to 100x slower in 9-b33 on a dual-socket machine. Adding back the check that value.length > 0 before entering the calculation loop recuperates the loss in both micro and xml.transform, but we're seeing as good or better number by simply guarding the store: Webrev: http://cr.openjdk.java.net/~redestad/8061254/webrev.00/ Bug: https://bugs.openjdk.java.net/browse/JDK-8061254 (confidential due to links to internal systems, sorry!) This additionally avoids the field store (and potential for pointless cache traffic) also on non-empty strings whose hash value happen to equals 0. Thanks! /Claes [1] See com.sun.org.apache.xml.internal.dtm.ref.ExpandedNameTable#getExpandedTypeID. [2] http://cr.openjdk.java.net/~redestad/8061254/expandedtypeid-micro.zip

-- Best regards, Sergey.

More information about the core-libs-dev mailing list