Re: RFR: 8264806: Remove the experimental JIT compiler [v2]

2021-04-27 Thread Doug Simon
On Tue, 27 Apr 2021 19:07:45 GMT, Igor Ignatyev  wrote:

> > I guess this should really be named `isJVMCICompilerEnabled` now and the 
> > `vm.graal.enabled` predicate renamed to `vm.jvmcicompiler.enabled` but 
> > maybe that's too big a change (or can be done later).
> 
> @dougxc, I don't think that we should (or even can) rename it to 
> `vm.jvmcicompiler.enabled`. although the value of the property is indeed 
> `true` whenever a jvmci compiler is enabled, it is used to filter out tests 
> that are incompatible with a particular compiler -- graal. so what we can do 
> is to change `requires/VMProps.java` to set `vm.graal.enabled` only if the 
> jvmci compiler is indeed graal, but on the other hand, we haven't heard 
> anyone complaining that the test coverage for their jvmci compiler has been 
> reduced, so I don't see much of the benefit here.
> 
> -- Igor

Yes, we should just it as is until a second JVMCI compiler shows up.

-

PR: https://git.openjdk.java.net/jdk/pull/3421


Re: RFR: 8264806: Remove the experimental JIT compiler [v2]

2021-04-27 Thread Igor Ignatyev
On Tue, 13 Apr 2021 09:30:23 GMT, Doug Simon  wrote:

> I guess this should really be named `isJVMCICompilerEnabled` now and the 
> `vm.graal.enabled` predicate renamed to `vm.jvmcicompiler.enabled` but maybe 
> that's too big a change (or can be done later).

@dougxc, I don't think that we should (or even can) rename it to 
`vm.jvmcicompiler.enabled`. although the value of the property is indeed `true` 
whenever a jvmci compiler is enabled, it is used to filter out tests that are 
incompatible with a particular compiler -- graal. so what we can do is to 
change `requires/VMProps.java` to set `vm.graal.enabled` only if the jvmci 
compiler is indeed graal, but on the other hand, we haven't heard anyone 
complaining that the test coverage for their jvmci compiler has been reduced, 
so I don't see much of the benefit here.

-- Igor

-

PR: https://git.openjdk.java.net/jdk/pull/3421


Re: RFR: 8264806: Remove the experimental JIT compiler [v5]

2021-04-26 Thread Vladimir Kozlov
> As part of [JEP 410](http://openjdk.java.net/jeps/410) remove code related to 
> Java-based JIT compiler (Graal) from JDK:
> 
> - `jdk.internal.vm.compiler` — the Graal compiler 
> - `jdk.internal.vm.compiler.management` — Graal's `MBean`
> - `test/hotspot/jtreg/compiler/graalunit` — Graal's unit tests
> 
> Remove Graal related code in makefiles.
> 
> Note, next two `module-info.java` files are preserved so that the JVMCI 
> module `jdk.internal.vm.ci` continues to build:
> 
> src/jdk.internal.vm.compiler/share/classes/module-info.java
> src/jdk.internal.vm.compiler.management/share/classes/module-info.java
> 
> 
> @AlanBateman suggested that we can avoid it by using Module API to export 
> packages at runtime . It requires changes in GraalVM's JVMCI too so I will 
> file followup RFE to implement it.
> 
> Tested hs-tier1-4

Vladimir Kozlov has updated the pull request with a new target base due to a 
merge or a rebase. The pull request now contains 12 commits:

 - Merge branch 'master' into JDK-8264806
 - Merge branch 'JDK-8264805' into JDK-8264806
 - Merge branch 'master' into JDK-8264805
 - Restore Compiler::isGraalEnabled()
 - Restore Graal Builder image makefile
 - Merge latest changes from branch 'JDK-8264805' into JDK-8264806
 - Address review comments
 - 8264806: Remove the experimental JIT compiler
 - Remove exports from Graal module to jdk.aot
 - Merge branch 'master' into JDK-8264805
 - ... and 2 more: https://git.openjdk.java.net/jdk/compare/694acedf...b1e9ba63

-

Changes: https://git.openjdk.java.net/jdk/pull/3421/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=3421=04
  Stats: 441532 lines in 2884 files changed: 0 ins; 441518 del; 14 mod
  Patch: https://git.openjdk.java.net/jdk/pull/3421.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/3421/head:pull/3421

PR: https://git.openjdk.java.net/jdk/pull/3421


Re: RFR: 8264806: Remove the experimental JIT compiler [v4]

2021-04-26 Thread Vladimir Kozlov
> As part of [JEP 410](http://openjdk.java.net/jeps/410) remove code related to 
> Java-based JIT compiler (Graal) from JDK:
> 
> - `jdk.internal.vm.compiler` — the Graal compiler 
> - `jdk.internal.vm.compiler.management` — Graal's `MBean`
> - `test/hotspot/jtreg/compiler/graalunit` — Graal's unit tests
> 
> Remove Graal related code in makefiles.
> 
> Note, next two `module-info.java` files are preserved so that the JVMCI 
> module `jdk.internal.vm.ci` continues to build:
> 
> src/jdk.internal.vm.compiler/share/classes/module-info.java
> src/jdk.internal.vm.compiler.management/share/classes/module-info.java
> 
> 
> @AlanBateman suggested that we can avoid it by using Module API to export 
> packages at runtime . It requires changes in GraalVM's JVMCI too so I will 
> file followup RFE to implement it.
> 
> Tested hs-tier1-4

Vladimir Kozlov has updated the pull request with a new target base due to a 
merge or a rebase. The pull request now contains 11 commits:

 - Merge branch 'JDK-8264805' into JDK-8264806
 - Merge branch 'master' into JDK-8264805
 - Restore Compiler::isGraalEnabled()
 - Restore Graal Builder image makefile
 - Merge latest changes from branch 'JDK-8264805' into JDK-8264806
 - Address review comments
 - 8264806: Remove the experimental JIT compiler
 - Remove exports from Graal module to jdk.aot
 - Merge branch 'master' into JDK-8264805
 - Merge branch 'master' into JDK-8264805
 - ... and 1 more: https://git.openjdk.java.net/jdk/compare/b524a81a...db7c9aaf

-

Changes: https://git.openjdk.java.net/jdk/pull/3421/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=3421=03
  Stats: 468493 lines in 3247 files changed: 2 ins; 468290 del; 201 mod
  Patch: https://git.openjdk.java.net/jdk/pull/3421.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/3421/head:pull/3421

PR: https://git.openjdk.java.net/jdk/pull/3421


Re: RFR: 8264806: Remove the experimental JIT compiler [v3]

2021-04-13 Thread Doug Simon
On Mon, 12 Apr 2021 22:10:06 GMT, Vladimir Kozlov  wrote:

>> As part of [JEP 410](http://openjdk.java.net/jeps/410) remove code related 
>> to Java-based JIT compiler (Graal) from JDK:
>> 
>> - `jdk.internal.vm.compiler` — the Graal compiler 
>> - `jdk.internal.vm.compiler.management` — Graal's `MBean`
>> - `test/hotspot/jtreg/compiler/graalunit` — Graal's unit tests
>> 
>> Remove Graal related code in makefiles.
>> 
>> Note, next two `module-info.java` files are preserved so that the JVMCI 
>> module `jdk.internal.vm.ci` continues to build:
>> 
>> src/jdk.internal.vm.compiler/share/classes/module-info.java
>> src/jdk.internal.vm.compiler.management/share/classes/module-info.java
>> 
>> 
>> @AlanBateman suggested that we can avoid it by using Module API to export 
>> packages at runtime . It requires changes in GraalVM's JVMCI too so I will 
>> file followup RFE to implement it.
>> 
>> Tested hs-tier1-4
>
> Vladimir Kozlov has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   Restore Compiler::isGraalEnabled()

Approved.

-

PR: https://git.openjdk.java.net/jdk/pull/3421


Re: RFR: 8264806: Remove the experimental JIT compiler [v2]

2021-04-13 Thread Vladimir Kozlov
On Tue, 13 Apr 2021 09:30:23 GMT, Doug Simon  wrote:

>> We would definitely like to be able to continue testing of GraalVM with the 
>> JDK set of jtreg tests. So keeping `Compiler::isGraalEnabled()` working like 
>> it does today is important.
>
>> @dougxc I restored Compiler::isGraalEnabled().
> 
> Thanks. I guess this should really be named `isJVMCICompilerEnabled` now and 
> the `vm.graal.enabled` predicate renamed to `vm.jvmcicompiler.enabled` but 
> maybe that's too big a change (or can be done later).

@dougxc
Renaming should be done separately. May be use RFE I filed: 
https://bugs.openjdk.java.net/browse/JDK-8265032

Do you approve these Graal removal changes?

-

PR: https://git.openjdk.java.net/jdk/pull/3421


Re: RFR: 8264806: Remove the experimental JIT compiler [v2]

2021-04-13 Thread Doug Simon
On Sun, 11 Apr 2021 10:25:47 GMT, Doug Simon  wrote:

>> Vladimir Kozlov has updated the pull request with a new target base due to a 
>> merge or a rebase. The incremental webrev excludes the unrelated changes 
>> brought in by the merge/rebase. The pull request contains three additional 
>> commits since the last revision:
>> 
>>  - Restore Graal Builder image makefile
>>  - Merge latest changes from branch 'JDK-8264805' into JDK-8264806
>>  - 8264806: Remove the experimental JIT compiler
>
> We would definitely like to be able to continue testing of GraalVM with the 
> JDK set of jtreg tests. So keeping `Compiler::isGraalEnabled()` working like 
> it does today is important.

> @dougxc I restored Compiler::isGraalEnabled().

Thanks. I guess this should really be named `isJVMCICompilerEnabled` now and 
the `vm.graal.enabled` predicate renamed to `vm.jvmcicompiler.enabled` but 
maybe that's too big a change (or can be done later).

-

PR: https://git.openjdk.java.net/jdk/pull/3421


Re: RFR: 8264806: Remove the experimental JIT compiler [v2]

2021-04-12 Thread Vladimir Kozlov
On Sun, 11 Apr 2021 10:25:47 GMT, Doug Simon  wrote:

>> Vladimir Kozlov has updated the pull request with a new target base due to a 
>> merge or a rebase. The incremental webrev excludes the unrelated changes 
>> brought in by the merge/rebase. The pull request contains three additional 
>> commits since the last revision:
>> 
>>  - Restore Graal Builder image makefile
>>  - Merge latest changes from branch 'JDK-8264805' into JDK-8264806
>>  - 8264806: Remove the experimental JIT compiler
>
> We would definitely like to be able to continue testing of GraalVM with the 
> JDK set of jtreg tests. So keeping `Compiler::isGraalEnabled()` working like 
> it does today is important.

@dougxc  I restored Compiler::isGraalEnabled().

-

PR: https://git.openjdk.java.net/jdk/pull/3421


Re: RFR: 8264806: Remove the experimental JIT compiler [v3]

2021-04-12 Thread Vladimir Kozlov
> As part of [JEP 410](http://openjdk.java.net/jeps/410) remove code related to 
> Java-based JIT compiler (Graal) from JDK:
> 
> - `jdk.internal.vm.compiler` — the Graal compiler 
> - `jdk.internal.vm.compiler.management` — Graal's `MBean`
> - `test/hotspot/jtreg/compiler/graalunit` — Graal's unit tests
> 
> Remove Graal related code in makefiles.
> 
> Note, next two `module-info.java` files are preserved so that the JVMCI 
> module `jdk.internal.vm.ci` continues to build:
> 
> src/jdk.internal.vm.compiler/share/classes/module-info.java
> src/jdk.internal.vm.compiler.management/share/classes/module-info.java
> 
> 
> @AlanBateman suggested that we can avoid it by using Module API to export 
> packages at runtime . It requires changes in GraalVM's JVMCI too so I will 
> file followup RFE to implement it.
> 
> Tested hs-tier1-4

Vladimir Kozlov has updated the pull request incrementally with one additional 
commit since the last revision:

  Restore Compiler::isGraalEnabled()

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/3421/files
  - new: https://git.openjdk.java.net/jdk/pull/3421/files/a246aaa6..9d6bd42c

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=3421=02
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=3421=01-02

  Stats: 25 lines in 1 file changed: 23 ins; 0 del; 2 mod
  Patch: https://git.openjdk.java.net/jdk/pull/3421.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/3421/head:pull/3421

PR: https://git.openjdk.java.net/jdk/pull/3421


Re: RFR: 8264806: Remove the experimental JIT compiler [v2]

2021-04-12 Thread Erik Joelsson
On Mon, 12 Apr 2021 17:18:36 GMT, Vladimir Kozlov  wrote:

>> make/common/Modules.gmk line 68:
>> 
>>> 66: 
>>> 67: # Filter out Graal specific modules
>>> 68: MODULES_FILTER += jdk.internal.vm.compiler
>> 
>> If we are unconditionally filtering out these modules, then why leave the 
>> module-info.java files in at all?
>
> We filter out because we can't build Graal anymore. But we need these 
> module-info.java files because JVMCI's module-info.java references them:
> https://github.com/openjdk/jdk/blob/master/src/jdk.internal.vm.ci/share/classes/module-info.java#L26
> 
> Otherwise we can't build JVMCI which we continue to support.
> 
> I filed followup RFE to implement Alan's suggestion to use Module  API which 
> will allow to remove these files later:
> https://bugs.openjdk.java.net/browse/JDK-8265091

Right, I thought I saw something about modules that Alan commented on, but 
couldn't find it. All good then.

-

PR: https://git.openjdk.java.net/jdk/pull/3421


Re: RFR: 8264806: Remove the experimental JIT compiler [v2]

2021-04-12 Thread Erik Joelsson
On Fri, 9 Apr 2021 22:26:40 GMT, Vladimir Kozlov  wrote:

>> As part of [JEP 410](http://openjdk.java.net/jeps/410) remove code related 
>> to Java-based JIT compiler (Graal) from JDK:
>> 
>> - `jdk.internal.vm.compiler` — the Graal compiler 
>> - `jdk.internal.vm.compiler.management` — Graal's `MBean`
>> - `test/hotspot/jtreg/compiler/graalunit` — Graal's unit tests
>> 
>> Remove Graal related code in makefiles.
>> 
>> Note, next two `module-info.java` files are preserved so that the JVMCI 
>> module `jdk.internal.vm.ci` continues to build:
>> 
>> src/jdk.internal.vm.compiler/share/classes/module-info.java
>> src/jdk.internal.vm.compiler.management/share/classes/module-info.java
>> 
>> 
>> @AlanBateman suggested that we can avoid it by using Module API to export 
>> packages at runtime . It requires changes in GraalVM's JVMCI too so I will 
>> file followup RFE to implement it.
>> 
>> Tested hs-tier1-4
>
> Vladimir Kozlov has updated the pull request with a new target base due to a 
> merge or a rebase. The incremental webrev excludes the unrelated changes 
> brought in by the merge/rebase. The pull request contains three additional 
> commits since the last revision:
> 
>  - Restore Graal Builder image makefile
>  - Merge latest changes from branch 'JDK-8264805' into JDK-8264806
>  - 8264806: Remove the experimental JIT compiler

Build changes look good.

-

Marked as reviewed by erikj (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/3421


Re: RFR: 8264806: Remove the experimental JIT compiler [v2]

2021-04-12 Thread Vladimir Kozlov
On Mon, 12 Apr 2021 16:18:32 GMT, Erik Joelsson  wrote:

>> Vladimir Kozlov has updated the pull request with a new target base due to a 
>> merge or a rebase. The incremental webrev excludes the unrelated changes 
>> brought in by the merge/rebase. The pull request contains three additional 
>> commits since the last revision:
>> 
>>  - Restore Graal Builder image makefile
>>  - Merge latest changes from branch 'JDK-8264805' into JDK-8264806
>>  - 8264806: Remove the experimental JIT compiler
>
> make/common/Modules.gmk line 68:
> 
>> 66: 
>> 67: # Filter out Graal specific modules
>> 68: MODULES_FILTER += jdk.internal.vm.compiler
> 
> If we are unconditionally filtering out these modules, then why leave the 
> module-info.java files in at all?

We filter out because we can't build Graal anymore. But we need these 
module-info.java files because JVMCI's module-info.java references them:
https://github.com/openjdk/jdk/blob/master/src/jdk.internal.vm.ci/share/classes/module-info.java#L26

Otherwise we can't build JVMCI which we continue to support.

I filed followup RFE to implement Alan's suggestion to use Module  API which 
will allow to remove these files later:
https://bugs.openjdk.java.net/browse/JDK-8265091

-

PR: https://git.openjdk.java.net/jdk/pull/3421


Re: RFR: 8264806: Remove the experimental JIT compiler [v2]

2021-04-12 Thread Erik Joelsson
On Fri, 9 Apr 2021 22:26:40 GMT, Vladimir Kozlov  wrote:

>> As part of [JEP 410](http://openjdk.java.net/jeps/410) remove code related 
>> to Java-based JIT compiler (Graal) from JDK:
>> 
>> - `jdk.internal.vm.compiler` — the Graal compiler 
>> - `jdk.internal.vm.compiler.management` — Graal's `MBean`
>> - `test/hotspot/jtreg/compiler/graalunit` — Graal's unit tests
>> 
>> Remove Graal related code in makefiles.
>> 
>> Note, next two `module-info.java` files are preserved so that the JVMCI 
>> module `jdk.internal.vm.ci` continues to build:
>> 
>> src/jdk.internal.vm.compiler/share/classes/module-info.java
>> src/jdk.internal.vm.compiler.management/share/classes/module-info.java
>> 
>> 
>> @AlanBateman suggested that we can avoid it by using Module API to export 
>> packages at runtime . It requires changes in GraalVM's JVMCI too so I will 
>> file followup RFE to implement it.
>> 
>> Tested hs-tier1-4
>
> Vladimir Kozlov has updated the pull request with a new target base due to a 
> merge or a rebase. The incremental webrev excludes the unrelated changes 
> brought in by the merge/rebase. The pull request contains three additional 
> commits since the last revision:
> 
>  - Restore Graal Builder image makefile
>  - Merge latest changes from branch 'JDK-8264805' into JDK-8264806
>  - 8264806: Remove the experimental JIT compiler

make/common/Modules.gmk line 68:

> 66: 
> 67: # Filter out Graal specific modules
> 68: MODULES_FILTER += jdk.internal.vm.compiler

If we are unconditionally filtering out these modules, then why leave the 
module-info.java files in at all?

-

PR: https://git.openjdk.java.net/jdk/pull/3421


Re: RFR: 8264806: Remove the experimental JIT compiler [v2]

2021-04-11 Thread Doug Simon
On Sat, 10 Apr 2021 17:41:05 GMT, Vladimir Kozlov  wrote:

>> Marked as reviewed by iignatyev (Reviewer).
>
> Thank you, Igor. I filed https://bugs.openjdk.java.net/browse/JDK-8265032

We would definitely like to be able to continue testing of GraalVM with the JDK 
set of jtreg tests. So keeping `Compiler::isGraalEnabled()` working like it 
does today is important.

-

PR: https://git.openjdk.java.net/jdk/pull/3421


Re: RFR: 8264806: Remove the experimental JIT compiler [v2]

2021-04-10 Thread Vladimir Kozlov
On Sat, 10 Apr 2021 16:47:45 GMT, Igor Ignatyev  wrote:

>> Vladimir Kozlov has updated the pull request with a new target base due to a 
>> merge or a rebase. The incremental webrev excludes the unrelated changes 
>> brought in by the merge/rebase. The pull request contains three additional 
>> commits since the last revision:
>> 
>>  - Restore Graal Builder image makefile
>>  - Merge latest changes from branch 'JDK-8264805' into JDK-8264806
>>  - 8264806: Remove the experimental JIT compiler
>
> Marked as reviewed by iignatyev (Reviewer).

Thank you, Igor. I filed https://bugs.openjdk.java.net/browse/JDK-8265032

-

PR: https://git.openjdk.java.net/jdk/pull/3421


Re: RFR: 8264806: Remove the experimental JIT compiler [v2]

2021-04-10 Thread Igor Ignatyev
On Sat, 10 Apr 2021 16:36:54 GMT, Vladimir Kozlov  wrote:

>> should we remove `sun.hotspot.code.Compiler::isGraalEnabled` method and 
>> update a few of its users accordingly?
>> what about `vm.graal.enabled` `@requires` property?
>
> @iignatev  If you think that I should clean tests anyway I will file follow 
> up RFE to do that.

> > should we remove `sun.hotspot.code.Compiler::isGraalEnabled` method and 
> > update a few of its users accordingly?
> > what about `vm.graal.enabled` `@requires` property?
> 
> Thank you, @iignatev for looking on changes.
> 
> I forgot to mention that `Compiler::isGraalEnabled()` returns always false 
> now. Because 94 tests uses `@requires !vm.graal.enabled` I don't want to 
> include them in these changes which are already very big. I am not sure if I 
> should modify tests if GraalVM group wants to run all these tests.

> If you think that I should clean tests anyway I will file follow up RFE to do 
> that.

changing  `Compiler::isGraalEnabled()` to always return false effectively makes 
these tests unrunnable for GraalVM group (unless they are keep the modified 
`sun/hotspot/code/Compiler` and/or `requires/VMProps` in their forks). on top 
of that, I foresee that there will be more tests incompatible w/ Graal yet 
given it won't be run/tested in OpenJDK, these tests won't be marked and hence 
will fail when run w/ Graal. so Graal people will have to either do marking 
themselves (I guess in both upstream and their fork) or maintain a list of 
inapplicable tests in a format that works best for their setup.

that's to say, I think we should clean up the tests, yet I totally agree there 
is no need to do it as part of this PR. we can discuss how to do it better for 
both OpenJDK and GraalVM folks in the follow-up RFE.

-

PR: https://git.openjdk.java.net/jdk/pull/3421


Re: RFR: 8264806: Remove the experimental JIT compiler [v2]

2021-04-10 Thread Igor Ignatyev
On Fri, 9 Apr 2021 22:26:40 GMT, Vladimir Kozlov  wrote:

>> As part of [JEP 410](http://openjdk.java.net/jeps/410) remove code related 
>> to Java-based JIT compiler (Graal) from JDK:
>> 
>> - `jdk.internal.vm.compiler` — the Graal compiler 
>> - `jdk.internal.vm.compiler.management` — Graal's `MBean`
>> - `test/hotspot/jtreg/compiler/graalunit` — Graal's unit tests
>> 
>> Remove Graal related code in makefiles.
>> 
>> Note, next two `module-info.java` files are preserved so that the JVMCI 
>> module `jdk.internal.vm.ci` continues to build:
>> src/jdk.internal.vm.compiler/share/classes/module-info.java
>> src/jdk.internal.vm.compiler.management/share/classes/module-info.java
>> 
>> @AlanBateman suggested that we can avoid it by using Module API to export 
>> packages at runtime . It requires changes in GraalVM's JVMCI too so I will 
>> file followup RFE to implement it.
>> 
>> Tested hs-tier1-4
>
> Vladimir Kozlov has updated the pull request with a new target base due to a 
> merge or a rebase. The incremental webrev excludes the unrelated changes 
> brought in by the merge/rebase. The pull request contains three additional 
> commits since the last revision:
> 
>  - Restore Graal Builder image makefile
>  - Merge latest changes from branch 'JDK-8264805' into JDK-8264806
>  - 8264806: Remove the experimental JIT compiler

Marked as reviewed by iignatyev (Reviewer).

-

PR: https://git.openjdk.java.net/jdk/pull/3421


Re: RFR: 8264806: Remove the experimental JIT compiler [v2]

2021-04-10 Thread Vladimir Kozlov
On Sat, 10 Apr 2021 15:38:11 GMT, Igor Ignatyev  wrote:

>> Vladimir Kozlov has updated the pull request with a new target base due to a 
>> merge or a rebase. The incremental webrev excludes the unrelated changes 
>> brought in by the merge/rebase. The pull request contains three additional 
>> commits since the last revision:
>> 
>>  - Restore Graal Builder image makefile
>>  - Merge latest changes from branch 'JDK-8264805' into JDK-8264806
>>  - 8264806: Remove the experimental JIT compiler
>
> should we remove `sun.hotspot.code.Compiler::isGraalEnabled` method and 
> update a few of its users accordingly?
> what about `vm.graal.enabled` `@requires` property?

@iignatev  If you think that I should clean tests anyway I will file follow up 
RFE to do that.

-

PR: https://git.openjdk.java.net/jdk/pull/3421


Re: RFR: 8264806: Remove the experimental JIT compiler [v2]

2021-04-10 Thread Vladimir Kozlov
On Sat, 10 Apr 2021 15:38:11 GMT, Igor Ignatyev  wrote:

> should we remove `sun.hotspot.code.Compiler::isGraalEnabled` method and 
> update a few of its users accordingly?
> what about `vm.graal.enabled` `@requires` property?

Thank you, @iignatev for looking on changes.

I forgot to mention that `Compiler::isGraalEnabled()` returns always false now. 
Because 94 tests uses `@requires !vm.graal.enabled` I don't want to include 
them in these changes which are already very big. I am not sure if I should 
modify tests if GraalVM group wants to run all these tests.

Unfortunately changes in `Compiler.java` are listed the last on `Files changed` 
tab and GitHub has trouble to load these big changes - it takes time to see 
them. Here `Compiler.java` chnges for review:
diff --git a/test/lib/sun/hotspot/code/Compiler.java 
b/test/lib/sun/hotspot/code/Compiler.java
index 99122bd93b8..71288ae4482 100644
--- a/test/lib/sun/hotspot/code/Compiler.java
+++ b/test/lib/sun/hotspot/code/Compiler.java
@@ -60,33 +60,10 @@ public class Compiler {
 /**
  * Check if Graal is used as JIT compiler.
  *
- * Graal is enabled if following conditions are true:
- * - we are not in Interpreter mode
- * - UseJVMCICompiler flag is true
- * - jvmci.Compiler variable is equal to 'graal'
- * - TieredCompilation is not used or TieredStopAtLevel is greater than 3
- * No need to check client mode because it set UseJVMCICompiler to false.
- *
- * @return true if Graal is used as JIT compiler.
+ * @return false because Graal is removed from JDK.
  */
 public static boolean isGraalEnabled() {
-Boolean useCompiler = WB.getBooleanVMFlag("UseCompiler");
-if (useCompiler == null || !useCompiler) {
-return false;
-}
-Boolean useJvmciComp = WB.getBooleanVMFlag("UseJVMCICompiler");
-if (useJvmciComp == null || !useJvmciComp) {
-return false;
-}
-
-Boolean tieredCompilation = WB.getBooleanVMFlag("TieredCompilation");
-Long compLevel = WB.getIntxVMFlag("TieredStopAtLevel");
-// if TieredCompilation is enabled and compilation level is <= 3 then 
no Graal is used
-if (tieredCompilation != null && tieredCompilation &&
-compLevel != null && compLevel <= 3) {
-return false;
-}
-return true;
+return false;
 }
 ```

-

PR: https://git.openjdk.java.net/jdk/pull/3421


Re: RFR: 8264806: Remove the experimental JIT compiler [v2]

2021-04-10 Thread Igor Ignatyev
On Fri, 9 Apr 2021 22:26:40 GMT, Vladimir Kozlov  wrote:

>> As part of [JEP 410](http://openjdk.java.net/jeps/410) remove code related 
>> to Java-based JIT compiler (Graal) from JDK:
>> 
>> - `jdk.internal.vm.compiler` — the Graal compiler 
>> - `jdk.internal.vm.compiler.management` — Graal's `MBean`
>> - `test/hotspot/jtreg/compiler/graalunit` — Graal's unit tests
>> 
>> Remove Graal related code in makefiles.
>> 
>> Note, next two `module-info.java` files are preserved so that the JVMCI 
>> module `jdk.internal.vm.ci` continues to build:
>> src/jdk.internal.vm.compiler/share/classes/module-info.java
>> src/jdk.internal.vm.compiler.management/share/classes/module-info.java
>> 
>> @AlanBateman suggested that we can avoid it by using Module API to export 
>> packages at runtime . It requires changes in GraalVM's JVMCI too so I will 
>> file followup RFE to implement it.
>> 
>> Tested hs-tier1-4
>
> Vladimir Kozlov has updated the pull request with a new target base due to a 
> merge or a rebase. The incremental webrev excludes the unrelated changes 
> brought in by the merge/rebase. The pull request contains three additional 
> commits since the last revision:
> 
>  - Restore Graal Builder image makefile
>  - Merge latest changes from branch 'JDK-8264805' into JDK-8264806
>  - 8264806: Remove the experimental JIT compiler

should we remove `sun.hotspot.code.Compiler::isGraalEnabled` method and update 
a few of its users accordingly?
what about `vm.graal.enabled` `@requires` property?

-

Changes requested by iignatyev (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/3421


Re: RFR: 8264806: Remove the experimental JIT compiler

2021-04-10 Thread Igor Ignatyev
On Fri, 9 Apr 2021 22:30:32 GMT, Vladimir Kozlov  wrote:

>> As part of [JEP 410](http://openjdk.java.net/jeps/410) remove code related 
>> to Java-based JIT compiler (Graal) from JDK:
>> 
>> - `jdk.internal.vm.compiler` — the Graal compiler 
>> - `jdk.internal.vm.compiler.management` — Graal's `MBean`
>> - `test/hotspot/jtreg/compiler/graalunit` — Graal's unit tests
>> 
>> Remove Graal related code in makefiles.
>> 
>> Note, next two `module-info.java` files are preserved so that the JVMCI 
>> module `jdk.internal.vm.ci` continues to build:
>> src/jdk.internal.vm.compiler/share/classes/module-info.java
>> src/jdk.internal.vm.compiler.management/share/classes/module-info.java
>> 
>> @AlanBateman suggested that we can avoid it by using Module API to export 
>> packages at runtime . It requires changes in GraalVM's JVMCI too so I will 
>> file followup RFE to implement it.
>> 
>> Tested hs-tier1-4
>
> Thankyou, @erikj79, for review. I restored code as you asked.
> For some reasons incremental webrev shows update only in Cdiffs.

none of the full webrevs seem to render even the list of changed files? is it 
just me?

-

PR: https://git.openjdk.java.net/jdk/pull/3421


Re: RFR: 8264806: Remove the experimental JIT compiler

2021-04-09 Thread Vladimir Kozlov
On Fri, 9 Apr 2021 17:35:11 GMT, Vladimir Kozlov  wrote:

> As part of [JEP 410](http://openjdk.java.net/jeps/410) remove code related to 
> Java-based JIT compiler (Graal) from JDK:
> 
> - `jdk.internal.vm.compiler` — the Graal compiler 
> - `jdk.internal.vm.compiler.management` — Graal's `MBean`
> - `test/hotspot/jtreg/compiler/graalunit` — Graal's unit tests
> 
> Remove Graal related code in makefiles.
> 
> Note, next two `module-info.java` files are preserved so that the JVMCI 
> module `jdk.internal.vm.ci` continues to build:
> src/jdk.internal.vm.compiler/share/classes/module-info.java
> src/jdk.internal.vm.compiler.management/share/classes/module-info.java
> 
> @AlanBateman suggested that we can avoid it by using Module API to export 
> packages at runtime . It requires changes in GraalVM's JVMCI too so I will 
> file followup RFE to implement it.
> 
> Tested hs-tier1-4

Thankyou, @erikj79, for review. I restored code as you asked.
For some reasons incremental webrev shows update only in Cdiffs.

-

PR: https://git.openjdk.java.net/jdk/pull/3421


Re: RFR: 8264806: Remove the experimental JIT compiler [v2]

2021-04-09 Thread Vladimir Kozlov
> As part of [JEP 410](http://openjdk.java.net/jeps/410) remove code related to 
> Java-based JIT compiler (Graal) from JDK:
> 
> - `jdk.internal.vm.compiler` — the Graal compiler 
> - `jdk.internal.vm.compiler.management` — Graal's `MBean`
> - `test/hotspot/jtreg/compiler/graalunit` — Graal's unit tests
> 
> Remove Graal related code in makefiles.
> 
> Note, next two `module-info.java` files are preserved so that the JVMCI 
> module `jdk.internal.vm.ci` continues to build:
> src/jdk.internal.vm.compiler/share/classes/module-info.java
> src/jdk.internal.vm.compiler.management/share/classes/module-info.java
> 
> @AlanBateman suggested that we can avoid it by using Module API to export 
> packages at runtime . It requires changes in GraalVM's JVMCI too so I will 
> file followup RFE to implement it.
> 
> Tested hs-tier1-4

Vladimir Kozlov has updated the pull request with a new target base due to a 
merge or a rebase. The incremental webrev excludes the unrelated changes 
brought in by the merge/rebase. The pull request contains three additional 
commits since the last revision:

 - Restore Graal Builder image makefile
 - Merge latest changes from branch 'JDK-8264805' into JDK-8264806
 - 8264806: Remove the experimental JIT compiler

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/3421/files
  - new: https://git.openjdk.java.net/jdk/pull/3421/files/8ff9b599..a246aaa6

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=3421=01
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=3421=00-01

  Stats: 102 lines in 12 files changed: 66 ins; 27 del; 9 mod
  Patch: https://git.openjdk.java.net/jdk/pull/3421.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/3421/head:pull/3421

PR: https://git.openjdk.java.net/jdk/pull/3421


Re: RFR: 8264806: Remove the experimental JIT compiler

2021-04-09 Thread Erik Joelsson
On Fri, 9 Apr 2021 17:35:11 GMT, Vladimir Kozlov  wrote:

> As part of [JEP 410](http://openjdk.java.net/jeps/410) remove code related to 
> Java-based JIT compiler (Graal) from JDK:
> 
> - `jdk.internal.vm.compiler` — the Graal compiler 
> - `jdk.internal.vm.compiler.management` — Graal's `MBean`
> - `test/hotspot/jtreg/compiler/graalunit` — Graal's unit tests
> 
> Remove Graal related code in makefiles.
> 
> Note, next two `module-info.java` files are preserved so that the JVMCI 
> module `jdk.internal.vm.ci` continues to build:
> src/jdk.internal.vm.compiler/share/classes/module-info.java
> src/jdk.internal.vm.compiler.management/share/classes/module-info.java
> 
> @AlanBateman suggested that we can avoid it by using Module API to export 
> packages at runtime . It requires changes in GraalVM's JVMCI too so I will 
> file followup RFE to implement it.
> 
> Tested hs-tier1-4

make/GraalBuilderImage.gmk line 1:

> 1: #

This file should not be removed. This outout image is this makefile produces is 
used as input to the separate GraalVM build.

make/Main.gmk line 444:

> 442: ))
> 443: 
> 444: $(eval $(call SetupTarget, graal-builder-image, \

Same as above, this needs to stay.

make/autoconf/spec.gmk.in line 895:

> 893: STATIC_LIBS_IMAGE_DIR := $(IMAGES_OUTPUTDIR)/$(STATIC_LIBS_IMAGE_SUBDIR)
> 894: 
> 895: # Graal builder image

Like above, this needs to stay.

-

PR: https://git.openjdk.java.net/jdk/pull/3421


RFR: 8264806: Remove the experimental JIT compiler

2021-04-09 Thread Vladimir Kozlov
As part of [JEP 410](http://openjdk.java.net/jeps/410) remove code related to 
Java-based JIT compiler (Graal) from JDK:

- `jdk.internal.vm.compiler` — the Graal compiler 
- `jdk.internal.vm.compiler.management` — Graal's `MBean`
- `test/hotspot/jtreg/compiler/graalunit` — Graal's unit tests

Remove Graal related code in makefiles.

Note, next two `module-info.java` files are preserved so that the JVMCI module 
`jdk.internal.vm.ci` continues to build:
src/jdk.internal.vm.compiler/share/classes/module-info.java
src/jdk.internal.vm.compiler.management/share/classes/module-info.java

@AlanBateman suggested that we can avoid it by using Module API to export 
packages at runtime . It requires changes in GraalVM's JVMCI too so I will file 
followup RFE to implement it.

Tested hs-tier1-4

-

Depends on: https://git.openjdk.java.net/jdk/pull/3398

Commit messages:
 - 8264806: Remove the experimental JIT compiler

Changes: https://git.openjdk.java.net/jdk/pull/3421/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=3421=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8264806
  Stats: 441620 lines in 2886 files changed: 0 ins; 441604 del; 16 mod
  Patch: https://git.openjdk.java.net/jdk/pull/3421.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/3421/head:pull/3421

PR: https://git.openjdk.java.net/jdk/pull/3421