Errors when tracing DinerPhilosophers

[clementbera]

I tried to trace with Dom DinerPhilosophers.

On my machine, I got errors with ByteBuffers: java.lang.NoSuchMethodError: java.nio.ByteBuffer.limit Same thing with a few other methods. It seems to be a jdk bug (ByteBuffers reimplements those methods with low level API which are unsupported with some drivers).

I've also add a partial evaluation error (It could not trace through isKindOf: since the recursion is too deep).

PR incoming.

[smarr]

@clementbera not sure this is needed. Did you discuss this with @daumayr? In his new implementation, we replaced ByteBuffer.

[clementbera]

At least the TruffleBoundary is needed in isKindOf, no?

[smarr]

Looking at SClass.isKindOf(.), I would advise against it, because it hides a performance issue.

I would prefer (following SClass.lookupClass):

VM.callerNeedsToBeOptimized(
        "This should not be on the fast path, specialization/caching needed?");

Generally, I would like if things are treated similar to how we do it in surrounding code.

It is also strange that isKindOf: becomes relevant for performance in the first place. What kind of code uses it?

[clementbera]

I don't remember what code we ran to make the problem happen. Dominik showed me the tracing from the dev branch, I have these 2 changes on my dev branch that were required to make it work on my machine, we traced DiningPhilosophers as a demo.

I can investigate why there was a problem with isKindOf:, but I might be better close this issue and do something else since Dom new tracing logic will replace this one and I am not sure I can reproduce.

[smarr]

Adding the

VM.callerNeedsToBeOptimized(
        "This should not be on the fast path, specialization/caching needed?");

would still be an improvement. Because there is indeed a PE issue (since it is recursive). And this communicates it more clearly.

[smarr]

Add the VM.callerNeedsToBeOptimized() as part of #251. @daumayr what's with the ByteBuffer change. Is that needed?

[daumayr]

The Bytebuffer changes are unnecessary, there was a Java 10 issue. Bytebuffers are gone in the new version.

[smarr]

Ok, can we do this as part of all the changes for the new actor tracing?

[daumayr]

I will integrate the necessary changes

[smarr]

Ok, thanks. I think we can close this issue once the changes are in.