Re: RFR: 8309671: Avoid using jvmci.Compiler property to determine if Graal is enabled
On Fri, 9 Jun 2023 12:14:46 GMT, Doug Simon wrote: > > Is JVMCI used by the Graal compiler only? > > So far this is true and will probably remain true for the foreseeable future. > However, the Right Thing to do long term is to add a > `jdk.test.whitebox.code.Compiler.uncommonTrapsHavePreciseBCIs()` method. I created https://bugs.openjdk.org/browse/JDK-8310346 for this enhancement. - PR Comment: https://git.openjdk.org/jdk/pull/14381#issuecomment-1598205886
Re: RFR: 8309671: Avoid using jvmci.Compiler property to determine if Graal is enabled
On Thu, 8 Jun 2023 17:14:39 GMT, Yudi Zheng wrote:
> HeapMonitor checks if System.getProperty("jvmci.Compiler") is graal and will
> not enforce checking line number derived from uncommon trap debug info.
> However, Graal does not set this property explicitly.
Thanks for the review!
-
PR Comment: https://git.openjdk.org/jdk/pull/14381#issuecomment-1588815689
Re: RFR: 8309671: Avoid using jvmci.Compiler property to determine if Graal is enabled
On Fri, 9 Jun 2023 05:26:59 GMT, Serguei Spitsyn wrote: > Is JVMCI used by the Graal compiler only? So far this is true and will probably remain true for the foreseeable future. However, the Right Thing to do long term is to add a `jdk.test.whitebox.code.Compiler.uncommonTrapsHavePreciseBCIs()` method. - PR Comment: https://git.openjdk.org/jdk/pull/14381#issuecomment-1584480496
Re: RFR: 8309671: Avoid using jvmci.Compiler property to determine if Graal is enabled
On Thu, 8 Jun 2023 17:14:39 GMT, Yudi Zheng wrote:
> HeapMonitor checks if System.getProperty("jvmci.Compiler") is graal and will
> not enforce checking line number derived from uncommon trap debug info.
> However, Graal does not set this property explicitly.
Looks good.
Is JVMCI used by the Graal compiler only?
Thanks,
Serguei
-
Marked as reviewed by sspitsyn (Reviewer).
PR Review: https://git.openjdk.org/jdk/pull/14381#pullrequestreview-1471250105
Re: RFR: 8309671: Avoid using jvmci.Compiler property to determine if Graal is enabled
On Thu, 8 Jun 2023 18:29:50 GMT, Yudi Zheng wrote:
>> test/hotspot/jtreg/serviceability/jvmti/HeapMonitor/MyPackage/HeapMonitor.java
>> line 257:
>>
>>> 255:
>>> 256: checkLines = !(enableJVMCI.getValue().equals("true")
>>> 257: && useJVMCICompiler.getValue().equals("true"));
>>
>> Is it possible to use `jdk.test.whitebox.code.Compiler.isGraalEnabled()`
>> here instead?
>
> To use whitebox I will have to update the config of every test here, which I
> think is too verbose:
>
> diff --git
> a/test/hotspot/jtreg/serviceability/jvmti/HeapMonitor/MyPackage/HeapMonitorStatIntervalTest.java
>
> b/test/hotspot/jtreg/serviceability/jvmti/HeapMonitor/MyPackage/HeapMonitorStatIntervalTest.java
> index e08454a4857..f086f744965 100644
> ---
> a/test/hotspot/jtreg/serviceability/jvmti/HeapMonitor/MyPackage/HeapMonitorStatIntervalTest.java
> +++
> b/test/hotspot/jtreg/serviceability/jvmti/HeapMonitor/MyPackage/HeapMonitorStatIntervalTest.java
> @@ -27,11 +27,15 @@ package MyPackage;
> /**
> * @test
> * @summary Verifies the JVMTI Heap Monitor sampling interval average.
> - * @build Frame HeapMonitor
> + * @library /test/lib
> + * @build Frame HeapMonitor jdk.test.whitebox.WhiteBox
> * @compile HeapMonitorStatIntervalTest.java
> * @requires vm.jvmti
> * @requires vm.compMode != "Xcomp"
> - * @run main/othervm/native -agentlib:HeapMonitorTest
> MyPackage.HeapMonitorStatIntervalTest
> + * @run driver jdk.test.lib.helpers.ClassFileInstaller
> jdk.test.whitebox.WhiteBox
> + * @run main/othervm/native -agentlib:HeapMonitorTest -Xbootclasspath/a:.
> + * -XX:+UnlockDiagnosticVMOptions -XX:+WhiteBoxAPI
> + * MyPackage.HeapMonitorStatIntervalTest
> */
Ok.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/14381#discussion_r1223447124
Re: RFR: 8309671: Avoid using jvmci.Compiler property to determine if Graal is enabled
On Thu, 8 Jun 2023 17:14:39 GMT, Yudi Zheng wrote:
> HeapMonitor checks if System.getProperty("jvmci.Compiler") is graal and will
> not enforce checking line number derived from uncommon trap debug info.
> However, Graal does not set this property explicitly.
Marked as reviewed by dnsimon (Committer).
-
PR Review: https://git.openjdk.org/jdk/pull/14381#pullrequestreview-1470601144
Re: RFR: 8309671: Avoid using jvmci.Compiler property to determine if Graal is enabled
On Thu, 8 Jun 2023 17:19:46 GMT, Doug Simon wrote:
>> HeapMonitor checks if System.getProperty("jvmci.Compiler") is graal and will
>> not enforce checking line number derived from uncommon trap debug info.
>> However, Graal does not set this property explicitly.
>
> test/hotspot/jtreg/serviceability/jvmti/HeapMonitor/MyPackage/HeapMonitor.java
> line 257:
>
>> 255:
>> 256: checkLines = !(enableJVMCI.getValue().equals("true")
>> 257: && useJVMCICompiler.getValue().equals("true"));
>
> Is it possible to use `jdk.test.whitebox.code.Compiler.isGraalEnabled()` here
> instead?
To use whitebox I will have to update the config of every test here, which I
think is too verbose:
diff --git
a/test/hotspot/jtreg/serviceability/jvmti/HeapMonitor/MyPackage/HeapMonitorStatIntervalTest.java
b/test/hotspot/jtreg/serviceability/jvmti/HeapMonitor/MyPackage/HeapMonitorStatIntervalTest.java
index e08454a4857..f086f744965 100644
---
a/test/hotspot/jtreg/serviceability/jvmti/HeapMonitor/MyPackage/HeapMonitorStatIntervalTest.java
+++
b/test/hotspot/jtreg/serviceability/jvmti/HeapMonitor/MyPackage/HeapMonitorStatIntervalTest.java
@@ -27,11 +27,15 @@ package MyPackage;
/**
* @test
* @summary Verifies the JVMTI Heap Monitor sampling interval average.
- * @build Frame HeapMonitor
+ * @library /test/lib
+ * @build Frame HeapMonitor jdk.test.whitebox.WhiteBox
* @compile HeapMonitorStatIntervalTest.java
* @requires vm.jvmti
* @requires vm.compMode != "Xcomp"
- * @run main/othervm/native -agentlib:HeapMonitorTest
MyPackage.HeapMonitorStatIntervalTest
+ * @run driver jdk.test.lib.helpers.ClassFileInstaller
jdk.test.whitebox.WhiteBox
+ * @run main/othervm/native -agentlib:HeapMonitorTest -Xbootclasspath/a:.
+ * -XX:+UnlockDiagnosticVMOptions -XX:+WhiteBoxAPI
+ * MyPackage.HeapMonitorStatIntervalTest
*/
-
PR Review Comment: https://git.openjdk.org/jdk/pull/14381#discussion_r122345
Re: RFR: 8309671: Avoid using jvmci.Compiler property to determine if Graal is enabled
On Thu, 8 Jun 2023 17:14:39 GMT, Yudi Zheng wrote:
> HeapMonitor checks if System.getProperty("jvmci.Compiler") is graal and will
> not enforce checking line number derived from uncommon trap debug info.
> However, Graal does not set this property explicitly.
test/hotspot/jtreg/serviceability/jvmti/HeapMonitor/MyPackage/HeapMonitor.java
line 257:
> 255:
> 256: checkLines = !(enableJVMCI.getValue().equals("true")
> 257: && useJVMCICompiler.getValue().equals("true"));
Is it possible to use `jdk.test.whitebox.code.Compiler.isGraalEnabled()` here
instead?
-
PR Review Comment: https://git.openjdk.org/jdk/pull/14381#discussion_r1223342262
RFR: 8309671: Avoid using jvmci.Compiler property to determine if Graal is enabled
HeapMonitor checks if System.getProperty("jvmci.Compiler") is graal and will
not enforce checking line number derived from uncommon trap debug info.
However, Graal does not set this property explicitly.
-
Commit messages:
- Avoid using jvmci.Compiler property to determine if Graal is enabled.
Changes: https://git.openjdk.org/jdk/pull/14381/files
Webrev: https://webrevs.openjdk.org/?repo=jdk=14381=00
Issue: https://bugs.openjdk.org/browse/JDK-8309671
Stats: 3 lines in 2 files changed: 0 ins; 2 del; 1 mod
Patch: https://git.openjdk.org/jdk/pull/14381.diff
Fetch: git fetch https://git.openjdk.org/jdk.git pull/14381/head:pull/14381
PR: https://git.openjdk.org/jdk/pull/14381
