New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[WIP] Add logging for UnresolvedMethod bug #9763
Conversation
Text txt = expectTextNode.execute(expression); | ||
return switch (txt.toString()) { | ||
case "ENABLE_LOG" -> { | ||
UnresolvedSymbol.logEnabled = true; | ||
yield EnsoContext.get(this).getNothing(); | ||
} | ||
case "DISABLE_LOG" -> { | ||
UnresolvedSymbol.logEnabled = false; | ||
yield EnsoContext.get(this).getNothing(); | ||
} | ||
default -> evalNode.execute(callerInfo, state, txt); | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we add a comment that this should be removed once the bug is fixed?
Text txt = expectTextNode.execute(expression); | ||
return switch (txt.toString()) { | ||
case "ENABLE_LOG" -> { | ||
UnresolvedSymbol.logEnabled = true; | ||
yield EnsoContext.get(this).getNothing(); | ||
} | ||
case "DISABLE_LOG" -> { | ||
UnresolvedSymbol.logEnabled = false; | ||
yield EnsoContext.get(this).getNothing(); | ||
} | ||
default -> evalNode.execute(callerInfo, state, txt); | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we add a comment that this should be removed once the bug is fixed?
The logging of the problematic test works, as can be seen in its output. So far, I have restarted the |
Could it be possible that adding the |
This fixes the PermanentBailoutException "too deep inlining" for some cases.
After 4 attempts, there is finally the UnresolvedMethod(==) bug reproduced in https://github.com/enso-org/enso/actions/runs/8802686851/job/24203973334?pr=9763#step:7:5817. Let's compare its output with previous successful attempt at https://github.com/enso-org/enso/actions/runs/8802686851/job/24191517691?pr=9763#step:7:4054 : The successful run:
The failed run:
The problem is in org.enso.interpreter.runtime.data.Type#allTypes method. In the failed run, it does not return Let's try running
and see if we can spot anything suspicious. There is a failed compilation with PermanentBailoutException "too deep inlining, probably caused by recursion": Log from Truffle compilation
Let's try to get rid of this PermanentBailoutException caused by recursion and see if it fixes the issue - 35a15e4
|
This reverts commit 35a15e4.
Hmm, what causes the Our type hierarchy is relatively flat - the most nesting we have is I think |
The
|
@JaroslavTulach Graal compiler expert advice needed. How would you get rid of the PermanentBailoutException that is thrown when compiling enso/engine/runtime/src/main/java/org/enso/interpreter/runtime/data/Type.java Lines 132 to 147 in 6d0a523
How to convince Truffle that it should not inline the |
Let's test my theory. I have added |
@radeusgd The "stack" that you see is actually not a "stack", it is an output from some Truffle-related inlining analysis. I believe there is nothing wrong with our algorithm in that method. I believe we just need to somehow convince Truffle to not inline that recursively. |
Ah ok, so I misunderstood this log after all. |
The error is still present even with |
@@ -126,7 +126,11 @@ add_specs suite_builder = | |||
|
|||
err = Error.throw "Error Value" | |||
1.is_a Error . should_be_false | |||
IO.println "ENABLE_LOG" | |||
Standard.Base.Runtime.Debug.eval "ENABLE_LOG" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for looking in to the Any.==
cannot be found error. It is really annoying.
@GregoryTravis was recently working on temporary enablement of stacktraces as part of
With that in place, this could should be:
Context.Dataflow_Stack_Trace.with_enabled <|
err.is_a Error . should_be_true
Of course, the code that produces the error shall log verbosely when the Context
is enabled.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think that PR would be any useful for this issue. Here, we have an uncaught Panic error that has a full stack trace already printed in https://github.com/enso-org/enso/actions/runs/8846897824/job/24293654909?pr=9763#step:7:6471.
I am surprised to see the Unresolved symbol <==> message header, but no logging about |
This reverts commit 55f381e.
@JaroslavTulach That is at the very beginning of the custom logging. There are many lines in the output beginning with "Resolving for ...", because the statement that disables the custom logging is not reached because of the unexpected panic (try searching for "DISABLE_LOG" in the output - you won't find that). |
The gist of a problem seems to be in the Type.allTypes method. In good scenario, for
In bad scenario, it returns these super types:
|
It has been two/three weeks since this transient was last encountered. I suspect that the #9796 PR "fixed" that by reordering the tests. Let's close this PR and keep in mind that it exists in case we encounter the |
Pull Request Description
Add logging to transient error
Reason: An unexpected panic was thrown: (No_Such_Method.Error Error UnresolvedSymbol<==>)
for example in https://github.com/enso-org/enso/actions/runs/8780930134/job/24091863122?pr=9751#step:7:4047 that happens approx more than 10% on job runs.Important Notes
Checklist
Please ensure that the following checklist has been satisfied before submitting the PR:
Scala,
Java,
and
Rust
style guides. In case you are using a language not listed above, follow the Rust style guide.
./run ide build
.