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. 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. /Claes
