@chrisdennis @henri-tremblay @akomakom Could you help to review this?

It's a bit strange. Normally we fallback to Reflection when unsafe doesn't work. So when would we need to use that? Also, there is this PR that should address the same thing in a more complicated way #64

We need this when run with --illegal-access=deny and without the workaround of adding -Djdk.attach.allowAttachSelf=true. Based on the Java 11 migration guide, --illegal-access=deny will be the default in a future java release. So we are migrating our program to be able to run with it (IMPALA-11260).

I just played around with #64. It still requires -Djdk.attach.allowAttachSelf=true. Even if I added this flag, I still see InaccessibleObjectException thrown in ObjectGraphWalker.getAllFields(). I might miss something. Please let me know if there are specific options needed.

It's a bit strange. Normally we fallback to Reflection when unsafe doesn't work.

I think we do this in estimating shallow sizes. But for getting field values (in ObjectGraphWalker), unsafe is a choice that we should try. JOL does the same: https://github.com/openjdk/jol/blob/5ec187a6d38443604221a63abec39c545d4d1674/jol-core/src/main/java/org/openjdk/jol/util/ObjectUtils.java#L118-L168

I can modify the code to try unsafe first. Just put it later to be consistent with the existing behavior.

Modified the title to make it more appropriate. We only try unsafe when it's available.

I understand the issue here... and the slight confusion too. There are two things going on here:

1. There is existing 'fallback' code for sizing individual objects in the graph: agent, unsafe, reflection. This PR doesn't touch that.
2. There is no existing fallback code for the reflection required to 'walk' the graph itself (before passing the individual objects to the sizing code). That is what is being addressed here.

There were some issues here though with how these changes interacted with the soft field caches. I therefore took the idea of using unsafe to navigate the references but implemented it in a different way in #64. @stiga-huang if you'd like to test that the updated version of that code works for you that would be very helpful. In the meantime I'll see what I can do to get that PR pushed through. The only real remaining issue is to decide if we want to directly use the byte-buddy agent, org instead improve our own loading code to be similarly capable.

Can we merge this for 0.4.1 and leave #64 for 0.5.0? We are waiting for a workable solution. I think we still need some time to review #64 .