On 4/8/19 11:25 AM, Claes Redestad wrote: > On 2019-04-08 10:56, Aleksey Shipilev wrote: >> 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. > > Sure, String::hashCode/hash_code locally becomes a bit more complex, but > I view this as being a net improvement on the total amount of special > handling we need to do for Strings and their hash codes.
I don't see it. The change *added* new handling for the flag in all those places we used to handle zero hash code, and then some. > While the performance gain on most real world use cases is likely to be > non-existent, there exists some past concerns with injecting zero hash > Strings into poorly implemented caches. This patch adds some defense-in- > depth to help avoid issues that could otherwise arise from use of > Strings as key in some hashing data structures. That does not make much sense to me: it is much easier to construct hashcode collisions rather than generating unique strings with zero hashcodes. Alternative hashing was there to mitigate that, and it would also handle zero hash attacks. -Aleksey
