Hi Naoto,

It seems you've missed the parentheses in :
Set<String> tagset = new HashSet<>(tokens.countTokens() * 4 + 2 / 3);
vs
Set<String> tagset = new HashSet<>((tokens.countTokens() * 4 + 2) / 3);

-Joe

On 5/5/2020 11:01 AM, naoto.s...@oracle.com wrote:
And here is the fix. Please review.

http://cr.openjdk.java.net/~naoto/8244459/webrev.00/

Naoto

On 5/5/20 10:25 AM, naoto.s...@oracle.com wrote:
Hi Peter,

You are correct. Thanks. I'll remove that initial value of 16.

Naoto

On 5/5/20 9:37 AM, Peter Levart wrote:
Hi Naoto,

On 4/30/20 12:18 AM, naoto.s...@oracle.com wrote:
Hello,

Please review this small fix to the following issue:

https://bugs.openjdk.java.net/browse/JDK-8244152

The proposed changeset is located at:

https://cr.openjdk.java.net/~naoto/8244152/webrev.00/

The hash map used there didn't have initial capacity, even though the exact numbers are known.


Well, it has to be calculated 1st (countTokens), but I guess this pays off when HashSet (the backing HashMap) does not have to be rehashed then.

The expression you use:

     Math.max((int)(tokens.countTokens() / 0.75f) + 1, 16)

...has a minimum value of 16. Why is that? 16 is just HashMap's default initialCapacity if not specified explicitly. But if you only want to store say 1 entry in the map, you can specify 2 as initialCapacity and HashMap will happily work for such case without resizing.


So you could just use:

     (int)(tokens.countTokens() / 0.75f) + 1

And even this expression is sometimes overshooting the minimal required value by 1 (when # of tokens is "exact" multiple of 0.75f, say 6). I think the following could be used to optimally pre-size the HashMap with default load factor 0.75:

     (tokens.countTokens() * 4 + 2) / 3


Regards, Peter


Naoto

Reply via email to