On Fri, 14 Apr 2023 13:28:40 GMT, Alan Bateman <al...@openjdk.org> wrote:

>> Can I please get a review of this change which removes the `java.compiler` 
>> system property? This addresses https://bugs.openjdk.org/browse/JDK-8041676.
>> 
>> A CSR has been filed for this change and is available at 
>> https://bugs.openjdk.org/browse/JDK-8305998. The CSR has more details about 
>> this proposed change, but to summarize, the `java.compiler` system property 
>> wasn't used in any practical way. With the recent removal of 
>> `java.lang.Compiler` interface from the JDK, the presence of this system 
>> property wasn't of any practical importance.
>> 
>> The commit in this PR, removes its reference from the 
>> `System.getProperties()` documentation and also removes the specific 
>> implementation from hotspot where it treated the values `NONE` and empty 
>> string in the absence of `-Xdebug` in a specific manner and considered it to 
>> be an instruction to run the application in interpreter-only mode. `-Xint` 
>> option has been around for this purpose, so removal of this `java.compiler` 
>> system property support from hotspot implementation wouldn't remove any 
>> usable feature for the applications.
>> 
>> Additionally, the hotspot implementation now logs a warning message when it 
>> detects the presence of `java.compiler` system property when `java` is 
>> launched. This warning message will be removed after a few releases.
>> 
>> The current JDK source has reference to the `java.compiler` system property 
>> in numerous NetBeans project build files. The usage of this system property 
>> in such files isn't necessary. However, this PR doesn't intend to cleanup 
>> those references, since it isn't clear which of those NetBeans projects are 
>> still relevant. I think we can use a separate PR to do that cleanup.
>> 
>> tier1, tier2, tier3 testing has been done with these changes and those tests 
>> have passed.
>
> src/hotspot/share/runtime/arguments.cpp line 1313:
> 
>> 1311:                 " use -Xint if you want to run the application in 
>> interpreted-only mode.");
>> 1312:     } else {
>> 1313:         warning("The java.compiler system property is obsolete and no 
>> longer supported.");
> 
> This looks okay although the warning for the empty value and "NONE" case will 
> probably wrap as it's very long. Today, -Djava.compiler=foo is ignored, no 
> warning, seems good to emit a warning about that too.

Hello Alan,

> This looks okay although the warning for the empty value and "NONE" case will 
> probably wrap as it's very long.

Agreed, it's a bit long. I can't think of a simpler message though. If anyone 
has a suggestion, I'll update accordingly.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/13475#discussion_r1166902101

Reply via email to