Hi,

I did a before/after run with the changes using Doug Lea's MapCheck microbenchmark [1]. I tested java.util.HashMap with Object, String, and Integer types.

It should be mentioned this was a quick check for any major performance changes: 2 iterations, run by hand on my own (relatively quiet) Mac. The changes are small. (A more serious run would be needed to accurately measure such a small performance difference, ideally using jmh, and a benchmark with more precise scoring.)

A brief summary of the scoring differences is below, by type of access and data type. The benchmark scores are based on the length of time to perform the operation, so a lower score is better, and a positive percent change means the operation took longer with the warnings cleanup changes.

---
MapCheck, java.util.HashMap, trials: 1000, size: 36864
%Change No Changes vs HashMap Warnings Cleanup

                     Object        String          Integer
Access Present       0.00%         0.00%           6.67%
Add Absent           0.00%         0.76%          -1.27%
Modify Present       0.00%        -1.16%           0.00%
Remove Present       0.00%         0.00%           0.00%
Search Absent        0.00%         2.00%           3.03%
Traverse entry       0.00%         0.00%           0.00%
Traverse key/val     0.00%         2.86%           0.00%
---

One might think the 6.6% increase for Access Present/Integer is cause for concern, but I think this is more a reflection of the scores being fairly coarse. Specifically, the two scores before the changes were 15 & 15, and with the changes were 15 & 17 (a mean of 16, 6.67% change from 15).

I think the change is fine to go back.

For reference, my spreadsheet of results is at [2].

Thanks,
-Brent

[1] http://gee.cs.oswego.edu/cgi-bin/viewcvs.cgi/jsr166/src/test/loops/MapCheck.java
[2] http://cr.openjdk.java.net/~bchristi/8022442/

On 8/6/13 3:23 PM, Dan Smith wrote:
On Aug 6, 2013, at 2:43 PM, Remi Forax <fo...@univ-mlv.fr> wrote:

On 08/06/2013 11:11 PM, Dan Smith wrote:
Please review this warnings cleanup.

Bug: http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=8022442 (not yet 
visible)
Webrev: http://cr.openjdk.java.net/~dlsmith/8022442/webrev.00/

—Dan

Hi Dan,
I've seen that you have introduce a common super interface for entry and tree 
node,
I suppose that  you check that there is no performance regression.
I wonder if an abstract class is not better than an interface because as far as 
I know
CHA implemented in hotspot doesn't work on interface
(but I may be wrong, there is perhaps a special optimization for arrays).

To make sure I understand: your concern is that an aastore will be more 
expensive when assigning to a KeyValueData[] than to an Object[] (or even to 
SomeOtherClass[])?

For what it's worth, all assignments to table[i] are statically known to be 
safe.  E.g.:

Entry<K,V> next = (Entry<K,V>) e.next;
...
table[i] = next;

So surely a smart VM only does the check once?

Here are some other things that might be concerns, but don't apply here:
- interface method invocations: there are no methods in the interface to invoke
- checkcast to an interface: all the casts are to concrete classes (Entry, 
TreeBin, TreeNode)

(There are some unchecked casts from KeyValueData to KeyValueData with 
different type parameters, but I assume these don't cause any checkcasts.)

—Dan

Reply via email to