On Tue, 15 Feb 2022 03:45:19 GMT, Stuart Marks <sma...@openjdk.org> wrote:

>>> The changes to j.l.Class and the EnumConstantDirectory test don't belong 
>>> here -- these are _uses_ of HashMap. This bug and fix should focus on 
>>> HashMap itself, to ensure that the cases in question allocate a table of 
>>> the right size.
>> 
>> A test will fail if not change codes there. Every pr should pass ci, so I 
>> have no choice.
>> 
>>> Are there any other maps that have this computation besides HashMap and 
>>> WeakHashMap?
>> 
>> Good question. Can I get a list of classes where I should check?(I guesd I 
>> shall start at LinkedHashMap and hash sets, but have no further ideas)
>> 
>>> There should be a regression test for this. It's probably sufficient to 
>>> base this on your original test program, which puts 12 entries into a 
>>> HashMap using a variety of techniques. It should assert that the table size 
>>> is 16 in all cases. Also, should there be a test case for WeakHashMap?
>> 
>> OK I will have a try... a hard part is how to read private field in class 
>> but I think I can find some clue in the existed tests.
>> 
>>> Also, I changed the summary of the bug report to be more precise. The PR 
>>> title will need to be changed to correspond to it. Thanks.
>> 
>> OK.
>
>> A test will fail if not change codes there. Every pr should pass ci, so I 
>> have no choice.
> 
> Hm, yes I recall in the preliminary email that there was some mention of a 
> test. However, the test seemed to use the same (incorrect) calculation, so 
> maybe the test needs to be fixed instead.
> 
>> Good question. Can I get a list of classes where I should check?(I guesd I 
>> shall start at LinkedHashMap and hash sets, but have no further ideas)
> 
> Offhand, the HashMap/LinkedHashMap and the corresponding Set classes, and 
> WeakHashMap, are the main places to look. IdentityHashMap and the Map.of() 
> implementations use a different organization so are probably unrelated. 
> ConcurrentHashMap is another obvious place; you might want to investigate 
> there, but depending on the fix (if any) we might want to handle it 
> separately. I'd search for "loadFactor" or "LOAD_FACTOR" and see if anything 
> else turns up.
> 
>> OK I will have a try... a hard part is how to read private field in class 
>> but I think I can find some clue in the existed tests.
> 
> There is an incantation in the test header to ensure that the java.base 
> module is opened for reflection. Let me know if you have trouble with it.

@stuart-marks done. please review again. thanks.

-------------

PR: https://git.openjdk.java.net/jdk/pull/7431

Reply via email to