Should GraphLayout/parseInstance use IdentityHashMap for GraphPathRecord's?

[Aleksey Shipilev]

Hi,

Sorry for extremely late reply. As you can see JOL development runs on idle cycles.

On 7/6/19 7:55 PM, Andy Fingerhut wrote:
> Thanks for developing JOL and making it so accurate!  I've recently been
> building a little utility library in Clojure on top of it, primarily for
> learning about Clojure and JVM implementation details.
> https://github.com/jafingerhut/cljol

I would appreciate if you could self-build the 0.10-SNAPSHOT and see if it passes the tests for your
project. That version should improve performance by a lot, due to the recent batch of fixes:
  https://hg.openjdk.java.net/code-tools/jol

> I was looking through the implementation of the object graph walking in
> GraphLayout's parseInstance method, and realized that while it might not be
> terribly likely, the TreeMap named 'addresses' could have 'addresses.put()'
> called on it for the same address multiple times, if a GC happens during
> graph walking.

Right. That was always a risk. It is somewhat mitigated by recent changes that try to snapshot
memory without doing anything else, and then process the data, taking the allocations there. While
some allocations still happen on snapshotting path, they should be much rarer.

We can still retry address snapshot, and newer code has a more convenient place to do it:
  https://bugs.openjdk.java.net/browse/CODETOOLS-7902585
  https://cr.openjdk.java.net/~shade/7902585/webrev.01/

I am still not very happy with that patch, but could you try and see if it alleviates the issue for you?

-- 
Thanks,
-Aleksey