I think the most benefit in this patch is the emptyString.hashCode()
speedup. By holding a boolean flag in the String object itself, there is
one less de-reference to be made on fast-path in case of empty string.
Which shows in microbenchmark and would show even more if code iterated
many different instances of empty strings that don't share the
underlying array invoking .hashCode() on them. Which, I admit, is not a
frequent case in practice, but hey, it is a speedup after all.
Regards, Peter
On 4/8/19 10:56 AM, Aleksey Shipilev wrote:
On 4/8/19 10:41 AM, Claes Redestad wrote:
by adding a bit to String that is true iff String.hash has been calculated as
being 0, we can get
rid of the corner case where such hash
codes are recalculated on every call.
Peter Levart came up with a elegant scheme for ensuring that we can keep
using non-volatile stores without explicit fencing and still reap the
benefits of this[1], and I've synced up the hotspot code that deals with
the String.hash value to mirror that logic.
Bug: https://bugs.openjdk.java.net/browse/JDK-8221836
Webrev: http://cr.openjdk.java.net/~redestad/8221836/open.01/
Since there exists small padding gaps in the current object layout of
strings (on all VM bitness and compressed oops varieties), adding this
boolean does not add any extra footprint per String instance.
Regardless, I think this change does not carry its weight. Introducing special
paths for handling
something as obscure as zero hash code, which then raises questions about
correctness (I had hard
time convincing myself that code is concurrency-safe), seems rather odd to me.
It is a sane
engineering tradeoff to make code more maintainable with accepting performance
hit in 2^(-32) of cases.
-Aleksey
