-
Notifications
You must be signed in to change notification settings - Fork 728
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
Assertion failure at openj9/runtime/compiler/x/codegen/J9TreeEvaluator.cpp:10779: !(comp->getOptions()->realTimeGC()) when PROD_WITH_ASSUMES is enabled #15440
Comments
@0xdaryl fyi |
This assert also fails in many other testlists in the There are 2 asserts fired from build 25390 that is very similar to the following:
There are 8 asserts fired in Build 25391 similar to the following:
and
There are 2 asserts fired in Build 25392:
and
There are 6 asserts fired in Build 25393 similar to the following:
and
|
This assert also appears in The are 4 asserts fired in Build 25387 similar to the following:
and
The 6 asserts fired in Build 25388 similar to the following:
and
|
Asserts from Issues: eclipse-openj9#15440, eclipse-openj9#15447, eclipse-openj9#15472, eclipse-openj9#15482 Signed-off-by: Manasha Vetrivelu <[email protected]>
@dylanjtuttle : please investigate |
This assertion inside the function Since metronome is a soft realtime GC policy, my guess is that If we can agree on that, it follows that either this assert is incorrect, and should possibly be removed, or we're not supposed to ever visit this function ( |
Update: it was surprisingly easy to figure out that we're not supposed to visit this function if the GC policy is metronome. I discovered the function EDIT: There are four methods that call one or both of these versions:
where an if-else call has the form: if (comp->getOptions()->realTimeGC())
{
// realtime call
}
else
{
// non-realtime call
} a guarded call has the form: if (!comp->getOptions()->realTimeGC())
{
// ...
// non-realtime call
// ...
} and an unsafe call has the form: // ...
// non-realtime call
// ... Since it seems like |
I'm trying out my potential fix, guarding the two unsafe calls to I won't be opening a PR with these changes yet, however, because I fail two test cases in |
My first idea to try and figure out why these two test cases were failing was to comment out the guarded calls to EDIT: everything else I wrote in this comment ended up being contradicted by my next comment, so I've deleted it. |
After removing the guarded realtime calls that I added, I ran sanity.functional on Jenkins and The "new" reloFlags assert that I mentioned above still fails. I'm suspicious that this may be caused by situations where we're using metronome and end up not generating any barrier at all because there's no guarded realtime call... So to summarize my discoveries so far, guarding the unsafe non-realtime calls fixes this failure but introduces a new one (which I'm referring to as "the reloFlags assert" to avoid confusion). Adding additional guarded realtime calls still fixes this failure and still introduces the reloFlags assert, but also causes test failures in |
This still doesn't really tell me anything about whether or not I should add the guarded realtime calls. |
I have a suspicion that you can't just simply call the "realtime" version when a realtime GC policy is in effect. There may be context differences that need to get sorted out. I could be wrong, but I never thought these two implementations were interchangeable like that. Let's take a step back though and address these problems one at at time. There are a lot of backtraces posted above and most without symbols, so let's first address the one that does have symbols. With that workaround in place, re-run and see what the backtrace is for other realtime asserts in the write barrier code. Let's address those when we see the context (please post here). |
Ok thanks Daryl, I'm working on that.
With that in mind, may I suggest we make the assertion message more descriptive?
|
In this previous comment, I mentioned how guarding the unsafe calls seemed to cause a new assertion failure that I called the reloFlags assert. I have reproduced the reloFlags failure in this new Jenkins job from today with a check to prevent inlining for realtime added, but also in this completely unrelated Jenkins job without that check. Thus, I am confident that the reloFlags failure is unrelated to any fixes made here, so I will be opening a new issue for that and commenting it out to continue investigating the effects caused by adding the check to prevent inlining for realtime. |
I've run a Jenkins job with one check added to Thus, making this change has "fixed" the assertion failure and has not caused or exposed any other test or assertion failures. |
I have now also run a Jenkins job on The job completed with no assertion or test failures. It is important to note that if there are any other non-fatal assertion failures caused by this change, we wouldn't have seen them in this run, but I think the only assert that really matters here is this one. @0xdaryl how should we proceed from here? I would say the proposed easy fix is doing its job. |
Closed as completed in #18027 |
In the
Special.system
test suite,The assert at
/home/jenkins/workspace/Build_JDK11_x86-64_linux_Personal/openj9/runtime/compiler/x/codegen/J9TreeEvaluator.cpp:10779: !(comp->getOptions()->realTimeGC())
failed when it was compiled with PROD_WITH_ASSUMES enabled. It was reproducible in all of the tests mentioned below.
It failed during the following tests:
DaaLoadTest_daa2_special_5m_11
DaaLoadTest_daa3_special_5m_11
MathLoadTest_all_special_5m_11
The platform used to build and test was x86-64_linux.
The link to the Jenkins Build.
The link to the Grinder Build.
The stack trace for each of the tests are:
DaaLoadTest_daa2_special_5m_11 twice with the trace:
and
DaaLoadTest_daa3_special_5m_11 with the trace:
MathLoadTest_all_special_5m_11 twice with the trace:
and
The text was updated successfully, but these errors were encountered: