Thanks for review Claes!
On 4/5/18 6:03 AM, Claes Redestad wrote:
Hi,
looks ok, but the cost/benefit ration of adding a standalone
regression test for every
such inefficiency seems dubious to me. Could we group these together,
somewhere?
Yes, I agree that this kind of tests should be combined with other
sanity checks.
However, I think it would be better organized, if the tests are placed
in the tested-class specific directory.
So, if we have some other sanity tests for VarHandle.AccessMode, then
they probably should be combined with this one to save resources.
Bonus startup points if you rewrote so that AccessMode.values() is
only called once,
since it clones the backing array.
Good point. I'll do that before pushing.
With kind regards,
Ivan
/Claes
On 2018-04-05 09:04, Ivan Gerasimov wrote:
Hello!
Yet another HashMap that can be preallocated more accurately.
Currently, the map is created as the following:
// Initial capacity of # values is sufficient to avoid
resizes
// for the smallest table size (32)
methodNameToAccessMode = new
HashMap<>(AccessMode.values().length);
Even though the comment suggests that no resizes of the table should
occur, in fact the threshold is calculated as 32 * 0.75 = 24, so when
24th element is inserted then the internal storage is reallocated and
the content of the table is reinserted.
Also, a missing @modules line is added to the regression test that
was pushed with the fix for JDK-8200696.
Would you please help review this?
BUGURL: https://bugs.openjdk.java.net/browse/JDK-8200788
WEBREV: http://cr.openjdk.java.net/~igerasim/8200788/00/webrev/
--
With kind regards,
Ivan Gerasimov
