[WIP] Add logging for UnresolvedMethod bug #9763
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Akirathan
added
the
CI: No changelog needed
radeusgd
reviewed
Apr 22, 2024
Comment on lines
+31
to
+42
| 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.
Can we add a comment that this should be removed once the bug is fixed?
radeusgd
reviewed
Apr 22, 2024
Comment on lines
+31
to
+42
| 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.
Can we add a comment that this should be removed once the bug is fixed?
2 tasks
|
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 compilationLet'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.
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.
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:
|
Akirathan
added
the
CI: Clean build required
This was referenced May 2, 2024
|
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 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.