-
Notifications
You must be signed in to change notification settings - Fork 15
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
[23.0] Sync with upstream #633
Conversation
(cherry picked from commit e27665f)
Some cleanups and additional sanity checks. (cherry picked from commit 6c0aa30)
(cherry picked from commit 1159814)
(cherry picked from commit 6bd08da)
(cherry picked from commit 6b05065)
(cherry picked from commit 65f4a3e)
…sue. PullRequest: graal/15912
… darwin-aarch64. PullRequest: graal/15910
…els. PullRequest: graal/15873
Disable the stripping of debug symbols from the generated native bits Closes oracle#6995 (cherry picked from commit 0162b22)
…f /json/list path. (GR-49664) (cherry picked from commit de48c84)
…ot reach here. PullRequest: graal/15886
… mx argument PullRequest: graal/15928
…8.2. PullRequest: js/2970
(cherry picked from commit 129fa26)
… called from a host call. (cherry picked from commit 0b14c1e) Adapt fix to 23.0
… entered context during close called from a host call PullRequest: graal/16015
(cherry picked from commit 514f292)
(cherry picked from commit 12e9a8b)
PullRequest: graal/15943
Some versions of `ld64` cannot deal with DIRECT8 relocations in the `.text` section. Approximately this is since version 820.1 (Xcode 14). Starting with Xcode15 beta3 the default linker has been replaced with "the new linker" (version 902.11) which does not suffer from this bug anymore. However, Xcode also ships a `ld-classic` which reassembles "the old linker" and is still affected by this bug (and it of course prints the same version number). See some more in-depth analysis of this ld64 bug in https://openradar.appspot.com/FB11942354 The workaround: Instead of emitting the address of a HostedMethod inlined, it will be put in the data section and thus requires a memory load to obtain it. (cherry picked from commit e30698b) resolve conflicts resolve errors backport fixup
(cherry picked from commit 6f02719) fix error add back method isAssociative() remove unused imports fix formatting issues
…nt range. PullRequest: graal/16151
PullRequest: graal/16231
…sThanNode PullRequest: graal/16170
…ge is queried via MemoryPoolMXBean. PullRequest: graal/16008
(cherry picked from commit 993a429)
…_info. PullRequest: graal/16227
PullRequest: graal/16264
…he header size. PullRequest: graal/16103
Looking at build failure |
15c098c
to
f3864b7
Compare
This was due to some conflicts with #575 Requested @roberttoyonaga's review in the corresponding lines. |
f3864b7
to
7fbf185
Compare
@@ -101,6 +102,6 @@ public boolean shouldEmit(long durationTicks) { | |||
|
|||
@Uninterruptible(reason = "Prevent races with VM operations that start/stop recording.", callerMustBe = true) | |||
private boolean shouldEmit0() { | |||
return SubstrateJVM.get().isRecording() && SubstrateJVM.get().isEnabled(this) && !SubstrateJVM.get().isCurrentThreadExcluded(); | |||
return SubstrateJVM.get().isRecording() && SubstrateJVM.get().isEnabled(this) && !JfrThreadLocal.isThreadExcluded(JavaThreads.getCurrentThreadOrNull()); |
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.
@roberttoyonaga can you please check if this is correct?
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.
Yes this is correct. But https://github.com/graalvm/mandrel/pull/633/files/f3864b7fbb9d325ae604fdb0ffab0e71506bf636#diff-5140a589f5128cadb659640373aafca28c66b801fef8c975d65c08fb9eded219R105 !SubstrateJVM.get().isCurrentThreadExcluded();
needs to be removed because now the checking is done in JfrThreadLocal
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.
oh it looks like you already removed it
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 remember there was some weird quirk that may have required isThreadExcluded
to be checked before isRecording
and isEnabled
. Let me quickly investigate to be sure.
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.
Ok this should be correct for 23.0.
The mandrel IT failure seems quarkusio/quarkus#37633 |
Indeed, the test no longer fails now that quarkusio/quarkus#37633 is merged, however with 23.0 we are getting:
which is not the case with 23.1 and 24.0-dev (probably related to the new class initialization strategy adopted in 23.1 https://github.com/oracle/graal/blob/master/substratevm/CHANGELOG.md#graalvm-for-jdk-21-internal-version-2310) We probably need to switch the corresponding netty classes to runtime initialize to make them compatible with 23.0. I will explore this following days. |
Fix in quarkusio/quarkus#37707 |
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.
OK. Tests seem alright.
It would be good for @roberttoyonaga to approve the jfr changes. |
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.
It would be good for @roberttoyonaga to approve the jfr changes.
It looks good to me. I tested it out on my machine as well. I just neglected to actually click approve.
Closes #618
I went through the changes and don't see anything concerning.