On 2019-04-08 17:10, Andrew Dinn wrote:
On 08/04/2019 15:55, Aleksey Shipilev wrote:
Why introduce this complexity to begin with? That's my argument. The
complication of this code gives
us almost nothing in return, why make the change? Without the compelling reason
to do it, all this
is just a runaway "hold my beer" micro-optimization exercise, which is fun and
instructional, but
should not be pushed to the actual JDK. In other words, just because we *can*
it does not follow
that we *should*.
I think that is a good argument. The vast majority of apps are not going
to see any performance hit from the relatively rare occurrence of zero
hashes. Those rare apps which see a lot of them will still only see a
marginal performance hit.
n.b. I say marginal because I believe that an app which sees enough zero
hashes for the effect not to be marginal (i.e.it is not doing much else)
is probably not an app but a Trojan horse of a (micro?) benchmark
masquerading as an app (timeo Danaos et lecti marcae ferentes -- pardon
my Latin and don't ask what those notches on the couch might mean ;-).
So, I agree with Aleksey that adding a potential maintenance headache
for this little gain is not the right trade-off.
First, I disagree strongly that this patch adds significant complexity
(especially after recent simplifications[1]) or that it risks increasing
maintenance headache down the line.
Secondly, I think the gain w.r.t. defense-in-depth is very real. Sure,
we have alt-hashing and similar counter-measures in various JDK
collection libraries, but that is unlikely to help every API or library
ever invented out there.
Third... well, the other performance gains are of course nice-to-
have - improvements to "".hashCode() and allowing String deduplication
to not have to filter out such Strings - but I agree that they are
likely mostly theoretical for anything real-world.
If this change then exposes a bug in some unexpected place elsewhere
(I can only guess what dangers lurks out there... unforeseen interaction
with a weaker memory model? some wonky JIT reordering?) then that might
even be for the better in the end. If/when that happens, we can opt to
back this out (or not) while addressing whatever issue we've unearthed.
/Claes
[1] http://cr.openjdk.java.net/~redestad/8221836/open.02/
