Re: RFR: 8281631: HashMap copy constructor and putAll can over-allocate table [v22]
On Mon, 21 Feb 2022 06:33:05 GMT, liach wrote: > Unfortunately I don't think there is any for now. @liach Yes. that is why I ask if there be evidence to show `(int) ((1 + m.size()) * 1.1)` be an optimization, but not otherwise. (IMO it is otherwise.) - PR: https://git.openjdk.java.net/jdk/pull/7431
Re: RFR: 8281631: HashMap copy constructor and putAll can over-allocate table [v22]
On Sun, 20 Feb 2022 20:30:29 GMT, XenoAmess wrote: >>> You should run benchmarks to see how bad the lookup performance degrades >>> after you saves memory used by the hash table. >> >> OK, would find some time for it. > >> > You should run benchmarks to see how bad the lookup performance degrades >> > after you saves memory used by the hash table. >> >> OK, would find some time for it. > > @liach which jmh test should I run by the way? > > Or is there some commandline tools for this? > > I found nothing related to IdentityHashMap under `test/micro` Unfortunately I don't think there is any for now. Maybe someone familiar with the identity hash map can comment on your changes to the expected map size - PR: https://git.openjdk.java.net/jdk/pull/7431
Re: RFR: 8281631: HashMap copy constructor and putAll can over-allocate table [v22]
On Sun, 20 Feb 2022 20:03:07 GMT, XenoAmess wrote: > > You should run benchmarks to see how bad the lookup performance degrades > > after you saves memory used by the hash table. > > OK, would find some time for it. @liach which jmh test should I run by the way? Or is there some commandline tools for this? I found nothing related to IdentityHashMap under `test/micro` - PR: https://git.openjdk.java.net/jdk/pull/7431
Re: RFR: 8281631: HashMap copy constructor and putAll can over-allocate table [v22]
On Sun, 20 Feb 2022 20:02:09 GMT, XenoAmess wrote: > You should run benchmarks to see how bad the lookup performance degrades > after you saves memory used by the hash table. OK, would find some time for it. - PR: https://git.openjdk.java.net/jdk/pull/7431
Re: RFR: 8281631: HashMap copy constructor and putAll can over-allocate table [v22]
On Sun, 20 Feb 2022 19:11:26 GMT, liach 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 regular hash map is using. This was > effectively trading some memory for better performance. You should run > benchmarks to see how bad the lookup performance degrades after you saves > memory used by the hash table. @liach 1. `* 1.1` don't seem a very reasonable number for making it non-dense 2. the main reason I argue about is it does not make the resize at `put` and the `map constructor` follow a same resize way. you create an IdentityHashMap `mapA`, put 19 key-value pairs, and build another IdentityHashMap using `mapB = new IdentityHashMap(mapA)`, and you can find mapB's table is twice larger than mapA's. That seems weird and nonsence. I can accept it if mapB's table's length is smaller or equal than mapA, but larger...why need to make it larger when we copy a map? - PR: https://git.openjdk.java.net/jdk/pull/7431
Re: RFR: 8281631: HashMap copy constructor and putAll can over-allocate table [v22]
> 8281631: HashMap copy constructor and putAll can over-allocate table XenoAmess has updated the pull request incrementally with one additional commit since the last revision: refine IdentityHashMap - Changes: - all: https://git.openjdk.java.net/jdk/pull/7431/files - new: https://git.openjdk.java.net/jdk/pull/7431/files/6d0ab0ef..e724cc04 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=7431=21 - incr: https://webrevs.openjdk.java.net/?repo=jdk=7431=20-21 Stats: 15 lines in 1 file changed: 0 ins; 11 del; 4 mod Patch: https://git.openjdk.java.net/jdk/pull/7431.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7431/head:pull/7431 PR: https://git.openjdk.java.net/jdk/pull/7431
