Let me try again... We exclude 0-hash Strings from archiving because we might write to the hash field, BUT the existing guard value.length > 0 ensures that won't happen for empty Strings ... so WHY were we excluding empty Strings from archiving?
On Mon, Apr 1, 2019 at 10:32 AM Claes Redestad <claes.redes...@oracle.com> wrote: > We could remove value.length > 0 and be net-neutral in #branches > for the general slow path (calculate the hash), but that would add a > branch to the emptyString.hashCode() _fast path_ (consistently add > ~0.5ns/op on my workstation with provided micro). We've been bitten > before that adding cost to "".hashCode() has effect on some benchmarks, > e.g., our internal XML implementation has some odd corner cases. > > Thus optimizing away a single branch from the calculating slow path (5% > for the empty string, quickly diminishing overhead with increased String > size) didn't seem worthwhile to me. > > /Claes > > On 2019-04-01 19:13, Martin Buchholz wrote: > > I'm confused. > > > > We have always had > > if (h == 0 && value.length > 0) { > > so we have already been special-casing empty strings (maybe we should > > stop?). > > > > Java has guaranteed that string literals are unique strings, but the > > empty string is not special in this regard. Archiving any string > > literal and restoring it later should be identity-preserving, no? > > > > Whenever we test or benchmark zero-hash Strings, there are 3 cases that > > should be covered - literal "", new String(), and (rare!) non-empty > > Strings that happen to hash to 0. > > > > We could avoid archiving those rare non-empty Strings that happen to > > hash to 0 at archive time if it saved us a branch at runtime. >