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