8205129: Remove java.lang.Compiler

[eirbjo]

Please help review this PR which suggests we remove the class java.lang.Compiler. The class seems non-functional for a decade, and was terminally deprecated in Java 9. Time has come to let it go.

---

Progress

• Change must be properly reviewed (1 review required, with at least 1 Reviewer)
• Change must not contain extraneous whitespace
• Commit message must refer to an issue
• Change requires CSR request JDK-8304458 to be approved

Issues

• JDK-8205129: Remove java.lang.Compiler
• JDK-8304458: Remove java.lang.Compiler (CSR)

Reviewers

• Alan Bateman (@AlanBateman - Reviewer)
• Jaikiran Pai (@jaikiran - Reviewer)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/13092/head:pull/13092
$ git checkout pull/13092

Update a local copy of the PR:
$ git checkout pull/13092
$ git pull https://git.openjdk.org/jdk.git pull/13092/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 13092

View PR using the GUI difftool:
$ git pr show -t 13092

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/13092.diff

Webrev

Link to Webrev Comment

[eirbjo]

/csr

[bridgekeeper]

👋 Welcome back eirbjo! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

[openjdk]

@eirbjo has indicated that a compatibility and specification (CSR) request is needed for this pull request.

@eirbjo please create a CSR request for issue JDK-8205129 with the correct fix version. This pull request cannot be integrated until the CSR request is approved.

[openjdk]

@eirbjo The following label will be automatically applied to this pull request:

• core-libs

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command.

[eirbjo]

/csr

[openjdk]

@eirbjo an approved CSR request is already required for this pull request.

[mlbridge]

Webrevs

• 00: Full (4b14fc57)

[eirbjo]

The actual change in this PR seems trivial (simply removes a file), so I guess it is more useful to review the CSR and the release note:

https://bugs.openjdk.org/browse/JDK-8304458 (CSR)
https://bugs.openjdk.org/browse/JDK-8304459 (Release note)

[AlanBateman]

For reference, here's the discussion from 2016 when it was proposed to deprecate j.l.Compiler, for removal. Tim Ellison from IBM engaged in that discussion as one of the replies asked about applications running on the J9 VM using this API.
https://mail.openjdk.org/pipermail/core-libs-dev/2016-September/043365.html

The discussion is also a reminder that -Djava.compiler=NONE is the equivalent of -Xint, I thought that system property was dropped a long time ago.

[eirbjo]

For reference, here's the discussion from 2016 when it was proposed to deprecate j.l.Compiler, for removal. Tim Ellison from IBM engaged in that discussion as one of the replies asked about applications running on the J9 VM using this API.

So it seems in 2016 IBM in was ok with an eventual removal of this class ("these APIs can be removed without grief")

However, I see that later in the thread Azul Systems raised concerns about a removal:

https://mail.openjdk.org/pipermail/core-libs-dev/2016-September/043585.html
https://mail.openjdk.org/pipermail/core-libs-dev/2016-September/043646.html

So to summarize: java.lang.Compiler.command is a key API for functionality
that we currently use, and that we leverage and need to remain in the
common (non platform specific) name space. While we can live with
deprecation as part of an API cleanup, this cleanup should result in a
suitable replacement API, and we would want a
non-platform-specific-namespace replacement to exist before the current API
is actually removed at any future date.

Would be good if someone from Azul Systems could give a 2023 assessment on removing this class?

The discussion is also a reminder that -Djava.compiler=NONE is the equivalent of -Xint, I thought that system property was dropped a long time ago.

It exists in System.getProperties Javadocs at least:

     * <tr><th scope="row">{@systemProperty java.compiler}</th>
     *     <td>Name of JIT compiler to use</td></tr>

Was this just for reference or are you suggesting that this PR is blocked by the 'java.compiler' somehow?

[eirbjo]

The discussion is also a reminder that -Djava.compiler=NONE is the equivalent of -Xint, I thought that system property was dropped a long time ago.

https://bugs.openjdk.org/browse/JDK-8041676 suggests to deprecate it, but from the last comment we still seem to be waiting for @dholmes-ora to get back from vacation on the 22nd :-)

[AlanBateman]

A search of 3M classes in 130k artifacts found just 8 references to this class. 6 of the 8 are versions of the same thing which seems to be an off-label usage in a class named net.openhft.chronicle.core.Jvm where it calls Compiler.enable() instead of Thread.onSpinWait on older JDK releases.

I remember the Azul mails from 2016 on this topic but I don't think they provided sufficient insight why an application would using this barely documented API. Anything relying on these methods to do something is probably very tied to the VM implementation.

-Djava.compiler=NONE pre-dates "full speed debugging" and I would think all traces can be removed now. Yes, there is still a reference to this system property in System.getProperties that could be looked at too. All of these things are low priority. The good thing about removing java.lang.Compiler is that is avoids new developers from stumbling on it.

[dholmes-ora]

The discussion is also a reminder that -Djava.compiler=NONE is the equivalent of -Xint, I thought that system property was dropped a long time ago.

https://bugs.openjdk.org/browse/JDK-8041676 suggests to deprecate it, but from the last comment we still seem to be waiting for @dholmes-ora to get back from vacation on the 22nd :-)

I guess I never heard back from @stuart-marks . :( I think JDK-8041676 needs to be resolved along side this issue.

[eirbjo]

I noticed there are quite a few uses of -Djava.compiler=none in the demo netbeans project/build files.

I have filed #13101 and #13098 to take care of those usages.

[stuart-marks]

I don't know how much interdependency there is between the java.compiler system property and the java.lang.Compiler class. On the library side I think they're independent. But it sounds like @dholmes-ora thinks they should be resolved at the same time, so maybe there some things going on between them inside the JVM.

The java.compiler property still seems to have some actual effect (see src/hotspot/share/runtime/arguments.cpp) so it shouldn't be removed immediately. Instead, it needs to be deprecated and its use should cause a warning to be issued. A comment in that file also mentions -Xdebug. The Hotspot help message for that says

-Xdebug           does nothing. Provided for backward compatibility.

which doesn't appear to be true, as -Xdebug seems to have an effect on the interpretation of the java.compiler property! Ugh. Well, this appears to be the only effect of -Xdebug, so maybe the eventual removal of the java.compiler property will make the help message become true.

Anyway it sounds to me like the next step is to deprecate the java.compiler property and to have setting it cause a warning to be issued. If @dholmes-ora thinks that removal of java.lang.Compiler should wait until after that, I'm ok with it.

P.S. I note that I am finally getting back to @dholmes-ora on this issue, and it is in fact after he has returned from vacation on the 22nd. :-)

[dholmes-ora]

I don't know how much interdependency there is between the java.compiler system property and the java.lang.Compiler class.
On the library side I think they're independent.

That may be true today but originally they were very much inter-dependent:

/**
 * The <code>Compiler</code> class is provided to support
 * Java-to-native-code compilers and related services. By design, the
 * <code>Compiler</code> class does nothing; it serves as a
 * placeholder for a JIT compiler implementation.
 * <p>
 * When the Java Virtual Machine first starts, it determines if the
 * system property <code>java.compiler</code> exists. (System
 * properties are accessible through <code>getProperty</code>  and ,
 * a method defined by the <code>System</code> class.) If so, it is
 * assumed to be the name of a library (with a platform-dependent
 * exact location and type); the <code>loadLibrary</code> method in
 * class <code>System</code> is called to load that library. If this
 * loading succeeds, the function named
 * <code>java_lang_Compiler_start()</code> in that library is called.
 * <p>
 * If no compiler is available, these methods do nothing.

The only part of that we have left today is that -Djava.compiler=NONE is an alias for -Xint mode.

JDK 9 effectively severed the class from the property by removing the above wording, so in that sense the two issues can be addressed separately. We should have handled the property deprecation back in JDK 9 too (not that there is an actual definition/process of deprecating system properties).

P.S I will try not to take any vacation until after this is resolved. :)

[dholmes-ora]

The only part of that we have left today is that -Djava.compiler=NONE is an alias for -Xint mode.

Edit: ... unless -Xdebug is specified in which case it is ignored.

[eirbjo]

The CSR for the removal of java.lang.Compiler is now looking for non-vacationing reviewers:

https://bugs.openjdk.org/browse/JDK-8304458

[AlanBateman]

I think you should be able to finalize the CSR JDK-8304458.

I think it would be good to remove "java.compiler" from the list of standard properties (table in System.getProperties) in the same release.

[eirbjo]

I think you should be able to finalize the CSR JDK-8304458.

Thanks for the gentle nudge, Alan.

I was actually not aware that finalization could be initiated by the assignee! I've seen the CSR lead handle this workflow previously. I pressed the "Finalize" button.

I think it would be good to remove "java.compiler" from the list of standard properties (table in System.getProperties) in the same release.

Yes, these seem closely related.

[eirbjo]

The CSR for this PR has now been approved.

@jddarcy comments in the CSR:

Per comments in the PR, please link to any follow-up bugs to remove java.compiler support.

My guess is the "comment" here refers to @AlanBateman comment above that it would be good to handle the removal of -Djava.compiler in the same release:

I think it would be good to remove "java.compiler" from the list of standard properties (table in System.getProperties) in the same release.

I have linked to JDK-8041676, even though that issue strictly describes the deprecation of -Djava.compiler, not the removal.

I think what I'm trying to express here is that it's ok to refer to JDK-8041676 for the task that deals with the java.compiler system property. And that we should take care of JDK-8041676 in the same release as this PR.

[eirbjo]

The CSR for this PR has been approved for a while now. The PR is still looking for reviewers.

I know the change itself is not a very exciting one to review, it is after all just a file being deleted. But we still need a reviewer to approve it :-)

Or maybe all reviewers are gone on easter vacation?

[openjdk]

@eirbjo This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8205129: Remove java.lang.Compiler

Reviewed-by: alanb, jpai

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 316 new commits pushed to the master branch:

• db174a1: 4825182: DefaultBoundedRangeModel.setMinimum() changes extent unnecessarily
• 969a6b9: 8304825: MacOS metal pipeline - window isn't painted if created during display sleep
• 9727685: 8305310: Calculate PublicKey from PrivateKey
• 50d7335: 8305696: (zipfs) Avoid redundant LinkedHashMap.containsKey call ZipFileSystem.makeParentDirs
• ec9d816: 6441827: Documentation mentions nonexistent NullReferenceException
• 9e20382: 8305721: add make compile-commands artifacts to .gitignore
• f45b01f: 8305766: ProblemList runtime/CompressedOops/CompressedClassPointers.java
• a8871f5: 8305407: ExternalSpecsWriter should ignore white-space differences in spec titles
• 6b2a86a: 8300257: C2: vectorization fails on some simple Memory Segment loops
• dc81603: 8305666: Add system property for fair AWT lock
• ... and 306 more: https://git.openjdk.org/jdk/compare/10f16746254ce62031f40ffb0f49f22e81cbe631...master

As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

As you do not have Committer status in this project an existing Committer must agree to sponsor your change. Possible candidates are the reviewers of this PR (@AlanBateman, @jaikiran) but any other Committer may sponsor as well.

➡️ To flag this PR as ready for integration with the above commit message, type /integrate in a new comment. (Afterwards, your sponsor types /sponsor in a new comment to perform the integration).

[eirbjo]

Good riddance.

/integrate

[openjdk]

@eirbjo
Your change (at version 4b14fc5) is now ready to be sponsored by a Committer.

[eirbjo]

This PR is daydreaming about a sponsor walking by.

[jaikiran]

Hello Eirik, I'll run this against our CI just to be sure and if things go fine, will sponsor this.

[jaikiran]

/sponsor

[openjdk]

Going to push as commit a8e3a2d.
Since your change was applied there have been 316 commits pushed to the master branch:

• db174a1: 4825182: DefaultBoundedRangeModel.setMinimum() changes extent unnecessarily
• 969a6b9: 8304825: MacOS metal pipeline - window isn't painted if created during display sleep
• 9727685: 8305310: Calculate PublicKey from PrivateKey
• 50d7335: 8305696: (zipfs) Avoid redundant LinkedHashMap.containsKey call ZipFileSystem.makeParentDirs
• ec9d816: 6441827: Documentation mentions nonexistent NullReferenceException
• 9e20382: 8305721: add make compile-commands artifacts to .gitignore
• f45b01f: 8305766: ProblemList runtime/CompressedOops/CompressedClassPointers.java
• a8871f5: 8305407: ExternalSpecsWriter should ignore white-space differences in spec titles
• 6b2a86a: 8300257: C2: vectorization fails on some simple Memory Segment loops
• dc81603: 8305666: Add system property for fair AWT lock
• ... and 306 more: https://git.openjdk.org/jdk/compare/10f16746254ce62031f40ffb0f49f22e81cbe631...master

Your commit was automatically rebased without conflicts.

[openjdk]

@jaikiran @eirbjo Pushed as commit a8e3a2d.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.

[eirbjo]

The Release Note for this PR did not receive any comments.

It would be good if a Reviewer can take a look at the release note and see if it is conforming to the desired style:

https://bugs.openjdk.org/browse/JDK-8304459

Thanks!