Re: RFR: 8281631: HashMap copy constructor and putAll can over-allocate table [v21]

2022-03-02 Thread XenoAmess
On Wed, 2 Mar 2022 01:44:24 GMT, Stuart Marks wrote: > I'm starting to look at this again. First, a quick note -- I don't think > there should be any IdentityHashMap changes here. That uses a completely > different internal structure and allocation policy, and it's kind of a > distraction

Re: RFR: 8281631: HashMap copy constructor and putAll can over-allocate table [v21]

2022-03-01 Thread Stuart Marks
On Sun, 20 Feb 2022 19:50:01 GMT, liach wrote: >> @liach Hi. please have a look at the latest commit. >> do you think it be better now? > > Oops, didn't notice there was this helpful `init` method. Does look much more > straightforward now. I'm starting to look at this again. First, a quick

Re: RFR: 8281631: HashMap copy constructor and putAll can over-allocate table [v21]

2022-02-20 Thread liach
On Sun, 20 Feb 2022 19:41:22 GMT, XenoAmess wrote: >> In fact, if we do worry about the performance of adding from maps, calling >> `map.forEach(this::put);` would be a better alternative both in concurrency >> (as the concurrent map itself takes charage) and object allocation-wise (no >>

Re: RFR: 8281631: HashMap copy constructor and putAll can over-allocate table [v21]

2022-02-20 Thread XenoAmess
On Sun, 20 Feb 2022 19:19:16 GMT, liach wrote: >> Imo you should just remove the `if (expectedSize == 0)` check than using >> this somewhat ugly trick to avoid calling `size()` twice (the second call is >> only used for this relatively useless fast-path, especially for the >> concurrent

Re: RFR: 8281631: HashMap copy constructor and putAll can over-allocate table [v21]

2022-02-20 Thread liach
On Sun, 20 Feb 2022 19:08:39 GMT, liach wrote: >>> @liach implementations `size()` seems O1, and returns a single int number >>> field, but it actually defers in some Map implementations. >> >> @liach for example, in ConcurrentSkipListMap and ConcurrentHashMap, `size()` >> function is far

Re: RFR: 8281631: HashMap copy constructor and putAll can over-allocate table [v21]

2022-02-20 Thread liach
On Sun, 20 Feb 2022 18:44:09 GMT, XenoAmess wrote: >> src/java.base/share/classes/java/util/IdentityHashMap.java line 281: >> >>> 279: * @throws NullPointerException if the specified map is null >>> 280: */ >>> 281: private IdentityHashMap(Map map, int >>> expectedSize) { >> >>

Re: RFR: 8281631: HashMap copy constructor and putAll can over-allocate table [v21]

2022-02-20 Thread liach
On Sun, 20 Feb 2022 18:08:24 GMT, XenoAmess wrote: > I don't thik it reasonable. or is there eveidence it be? If this map is too dense, there may be a lot of hash collisions, and the lookup performance would decrease because this hashmap is linear probe than red-black tree buckets like the

Re: RFR: 8281631: HashMap copy constructor and putAll can over-allocate table [v21]

2022-02-20 Thread XenoAmess
On Sun, 20 Feb 2022 18:20:27 GMT, liach wrote: >> XenoAmess has updated the pull request incrementally with three additional >> commits since the last revision: >> >> - refine test >> - 1. optimize IdentityHashMap that when calling ::new(Map), do not call >> map.size() twice but once. >>

Re: RFR: 8281631: HashMap copy constructor and putAll can over-allocate table [v21]

2022-02-20 Thread XenoAmess
On Sun, 20 Feb 2022 18:38:35 GMT, XenoAmess wrote: >> src/java.base/share/classes/java/util/IdentityHashMap.java line 281: >> >>> 279: * @throws NullPointerException if the specified map is null >>> 280: */ >>> 281: private IdentityHashMap(Map map, int >>> expectedSize) { >> >>

Re: RFR: 8281631: HashMap copy constructor and putAll can over-allocate table [v21]

2022-02-20 Thread XenoAmess
On Sun, 20 Feb 2022 18:20:27 GMT, liach wrote: >> XenoAmess has updated the pull request incrementally with three additional >> commits since the last revision: >> >> - refine test >> - 1. optimize IdentityHashMap that when calling ::new(Map), do not call >> map.size() twice but once. >>

Re: RFR: 8281631: HashMap copy constructor and putAll can over-allocate table [v21]

2022-02-20 Thread liach
On Sun, 20 Feb 2022 18:08:37 GMT, XenoAmess wrote: >> 8281631: HashMap copy constructor and putAll can over-allocate table > > XenoAmess has updated the pull request incrementally with three additional > commits since the last revision: > > - refine test > - 1. optimize IdentityHashMap that

Re: RFR: 8281631: HashMap copy constructor and putAll can over-allocate table [v21]

2022-02-20 Thread XenoAmess
On Sun, 20 Feb 2022 18:03:07 GMT, XenoAmess wrote: >> XenoAmess has updated the pull request incrementally with three additional >> commits since the last revision: >> >> - refine test >> - 1. optimize IdentityHashMap that when calling ::new(Map), do not call >> map.size() twice but once.

Re: RFR: 8281631: HashMap copy constructor and putAll can over-allocate table [v21]

2022-02-20 Thread XenoAmess
On Sun, 20 Feb 2022 18:04:50 GMT, XenoAmess wrote: >> 8281631: HashMap copy constructor and putAll can over-allocate table > > XenoAmess has updated the pull request incrementally with three additional > commits since the last revision: > > - refine test > - 1. optimize IdentityHashMap that

Re: RFR: 8281631: HashMap copy constructor and putAll can over-allocate table [v21]

2022-02-20 Thread XenoAmess
> 8281631: HashMap copy constructor and putAll can over-allocate table XenoAmess has updated the pull request incrementally with three additional commits since the last revision: - refine test - 1. optimize IdentityHashMap that when calling ::new(Map), do not call map.size() twice but once.