RFR: 8221836: Avoid recalculating String.hash when zero
claes.redestad at oracle.com
Mon Apr 8 21:19:22 UTC 2019
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) 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.
More information about the core-libs-dev