Re: [15] RFR (xs) 8046362 IdentityHashMap.hash comments should be clarified

2020-02-11 Thread Stuart Marks
Hi Roger, I don't want to have a detailed discussion of the performance tradeoffs in the comments. My comments changes are intended to address two specific points: the "multiply by -127" phrase and the arrangement of alternating keys and values in the array. Both have been sources of

Re: [15] RFR (xs) 8046362 IdentityHashMap.hash comments should be clarified

2020-02-11 Thread Martin Buchholz
On Mon, Feb 10, 2020 at 4:09 PM Stuart Marks wrote: > > Should I proceed with pushing my comments changes (which I've appended > below for > convenience)? > You already have my approval!

Re: [15] RFR (xs) 8046362 IdentityHashMap.hash comments should be clarified

2020-02-11 Thread Roger Riggs
Hi Stuart, The implNote compares the performance impact without mentioning that HashMap uses 'equals()' vs '==' which might have as big an impact on performance as sequential vs chained references. Detailed performance comparisons are very difficult given the factors that impact performance,

Re: [15] RFR (xs) 8046362 IdentityHashMap.hash comments should be clarified

2020-02-10 Thread Stuart Marks
On 2/10/20 7:52 AM, Martin Buchholz wrote: On Fri, Feb 7, 2020 at 2:47 PM David Holmes wrote: Hi Martin, On 8/02/2020 3:10 am, Martin Buchholz wrote: I explored System.identityHashCode (see below). System.identityHashCode is a VM call that is potentially very expensive. In the

Re: [15] RFR (xs) 8046362 IdentityHashMap.hash comments should be clarified

2020-02-10 Thread Martin Buchholz
On Fri, Feb 7, 2020 at 2:47 PM David Holmes wrote: > Hi Martin, > > On 8/02/2020 3:10 am, Martin Buchholz wrote: > > I explored System.identityHashCode (see below). > > System.identityHashCode is a VM call that is potentially very expensive. > In the worst-case it triggers monitor inflation. >

Re: [15] RFR (xs) 8046362 IdentityHashMap.hash comments should be clarified

2020-02-07 Thread David Holmes
Hi Martin, On 8/02/2020 3:10 am, Martin Buchholz wrote: I explored System.identityHashCode (see below). System.identityHashCode is a VM call that is potentially very expensive. In the worst-case it triggers monitor inflation. Cheers, David They appear to be high quality, pre-mixed ints

Re: [15] RFR (xs) 8046362 IdentityHashMap.hash comments should be clarified

2020-02-07 Thread Martin Buchholz
If a "modern" textbook recommends linear probing for memory locality, it would be nice to include a reference. I think it's OK as long as we have good mixing and occupancy doesn't get too high - 2/3 threshold seems good to me.

Re: [15] RFR (xs) 8046362 IdentityHashMap.hash comments should be clarified

2020-02-07 Thread Martin Buchholz
I explored System.identityHashCode (see below). They appear to be high quality, pre-mixed ints (not just addresses). I suggest that IdentityHashMap simply trust System.identityHashCode. I'm surprised at any attempt to defend against high bits being zero. I'm far more worried about low bits, as

Re: [15] RFR (xs) 8046362 IdentityHashMap.hash comments should be clarified

2020-02-07 Thread Doug Lea
On 2/6/20 12:31 PM, Martin Buchholz wrote: > lgtm > > Knuth didn't like linear probing, but memory locality became far more > important over time, favoring linear probing. (Or variants, like cuckoo, but they are unlikely to be improvements here.) > > I don't get what hash() is doing with the

Re: [15] RFR (xs) 8046362 IdentityHashMap.hash comments should be clarified

2020-02-06 Thread Martin Buchholz
lgtm Knuth didn't like linear probing, but memory locality became far more important over time, favoring linear probing. I don't get what hash() is doing with the low bits. Maybe we want something like mix32. On Thu, Feb 6, 2020 at 1:01 AM Andrew Haley wrote: > On 1/28/20 11:41 PM, Stuart

Re: [15] RFR (xs) 8046362 IdentityHashMap.hash comments should be clarified

2020-02-06 Thread Andrew Haley
On 1/28/20 11:41 PM, Stuart Marks wrote: > As an aside, I don't know whether the linear-probing (open > addressed) arrangement of IdentityHashMap is in fact faster than the > separate chaining approach of HashMap. Perhaps investigating that > could be a side project for someone. It always has

[15] RFR (xs) 8046362 IdentityHashMap.hash comments should be clarified

2020-01-28 Thread Stuart Marks
Hi all, I was looking through the bug database and I got nerd-sniped this bug report, which requests clarifying a comment in IdentityHashMap. Once I figured out what was going on, I decided I might as well just go ahead and fix it. This changeset improves (I hope) the comment in the