RE: RFR: 8144062: Move jdk.Version to java.lang.Runtime.Version

2016-05-19 Thread Iris Clark
Hi, Mandy.

Thank you so much for pushing the changesets [0,1] for this bug.

Regards,
Iris

[0]: http://hg.openjdk.java.net/jdk9/dev/jdk/rev/3976fadb091d
[1]: http://hg.openjdk.java.net/jdk9/dev/langtools/rev/2a49d47a37d8


Re: RFR: 8144062: Move jdk.Version to java.lang.Runtime.Version

2016-05-18 Thread Mandy Chung

> On May 13, 2016, at 4:20 PM, Iris Clark  wrote:
> 
> Hi.
> 
> Reviving this work from a few months back.
> 
> Please review the following changes to move jdk.Version to 
> jdk.lang.Runtime.Version.
> 
> Bug
> 
>8144062: Move jdk.Version to java.lang.Runtime.Version
>https://bugs.openjdk.java.net/browse/JDK-8144062
> 
> webrev
> 
>http://cr.openjdk.java.net/~iris/verona/8144062/webrev.1/


The change looks fine.  Minor comments:

1178  * @throws  IllegalArgumentException
1179  *  If the given string cannot be interpreted as a valid
1180  *  version
1185  * @throws  NumberFormatException
1186  *  If an element of the version number or the build number
1187  *  cannot be represented as an {@link Integer}

It’s okay to specify @throws NumberFormatException while @throws IAE (merging 
the description) should be adequate (the implementation stays the same).  
Something you can consider in the future.

1189  * @return  This version

It seems clearer to say "@return the Version of the given string” (this is a 
static method and no “This version”)

Mandy

RE: RFR: 8144062: Move jdk.Version to java.lang.Runtime.Version

2016-05-16 Thread Iris Clark
Hi, Remi.

Thanks for taking the time to review this change.

> java.lang.Runtime.Version is used during the boot process

I don’t think that Runtime.Version is used during boot because
I'm not seeing it loaded with a small test program invoked with
"java -verbose:class Hi".  In fact, I'm not seeing a difference
in the number of loaded classes between promoted build 118 and
my build for 8144062 (based on jdk9/dev).  See appended stats.

If my test program is in a JAR file, then more classes are
loaded including Runtime.Version; however the equivalent number
of classes are loaded before my changes too.

The performance problem identified by the following bug should
resolve this issue:

  8150678: JarFile stream() and entries(0 methods need performance
enhancement
  https://bugs.openjdk.java.net/browse/JDK-8150678

Regards,
Iris

-
$ cat Hi.java
public class Hi {
public static void main(String ... args) {
System.out.println("hi");
System.exit(42);
}
$ wc Hi.ver*
   501   2000  39758 Hi.verbose-118
   576   2300  45915 Hi.verbose-118-jar
   501   2000  39734 Hi.verbose-8144062
   576   2300  45905 Hi.verbose-8144062-jar
  2154   8600 171312 total

-Original Message-
From: Remi Forax [mailto:fo...@univ-mlv.fr] 
Sent: Friday, May 13, 2016 4:32 PM
To: Iris Clark
Cc: Java Core Libs; compiler-...@openjdk.java.net; verona-...@openjdk.java.net
Subject: Re: RFR: 8144062: Move jdk.Version to java.lang.Runtime.Version

Hi Iris,
is there a way to avoid to use regex when parsing the version ?

java.lang.Runtime.Version is used during the boot process, so now every Java 
programs loads a bunch of classes related to java.util.regex even if they do 
not use any regex or use another regex engine (like Nashorn or JRuby).

regards,
Rémi

- Mail original -
> De: "Iris Clark" <iris.cl...@oracle.com>
> À: "Java Core Libs" <core-libs-dev@openjdk.java.net>, 
> compiler-...@openjdk.java.net
> Cc: verona-...@openjdk.java.net
> Envoyé: Samedi 14 Mai 2016 01:20:23
> Objet: RFR: 8144062: Move jdk.Version to java.lang.Runtime.Version
> 
> Hi.
> 
> Reviving this work from a few months back.
> 
> Please review the following changes to move jdk.Version to 
> java.lang.Runtime.Version.
> 
> Bug
> 
> 8144062: Move jdk.Version to java.lang.Runtime.Version
> https://bugs.openjdk.java.net/browse/JDK-8144062
> 
> webrev
> 
> http://cr.openjdk.java.net/~iris/verona/8144062/webrev.1/
> 
> When jdk.Version was initially pushed in jdk-9+1-5, it was Improperly 
> exported by java.base.  After exploring a few options, the best choice 
> was to move jdk.Version to java.lang.Runtime.Version (a nested class 
> of Runtime).  By making Version an SE API, it may be exported by the 
> java.base module.
> 
> As part of the move, a limited number of chnages were made to the 
> Version class:
> 
>   - Change package name and class declaration (to static)
>   - Eliminate use of "JDK" when describing a Java SE API
>   - Initial clarifications related to zeros (trailing vs.
> Internal components)
>   - Small typographical and grammatical enhancements
>   - Indentation
> 
> The complete Runtime.Version specification is available here:
> 
>   
> http://cr.openjdk.java.net/~iris/verona/8144062/doc.1/java/lang/Runtim
> e.Version.html
> 
> The old jdk.Version.current() was replaced with Runtime.version().
> 
> In System.getProperties(), we indicate which version-related system 
> properties may be supported by Runtime.Version.
> 
> The remaining jdk and langtools file changes are all side-effects of 
> changing jdk.Version.current() to Runtime.version().
> 
> Thanks,
> Iris
> 


Re: RFR: 8144062: Move jdk.Version to java.lang.Runtime.Version

2016-05-13 Thread Remi Forax
Hi Iris,
is there a way to avoid to use regex when parsing the version ?

java.lang.Runtime.Version is used during the boot process, so now every Java 
programs loads a bunch of classes related to java.util.regex even if they do 
not use any regex or use another regex engine (like Nashorn or JRuby).

regards,
Rémi

- Mail original -
> De: "Iris Clark" 
> À: "Java Core Libs" , 
> compiler-...@openjdk.java.net
> Cc: verona-...@openjdk.java.net
> Envoyé: Samedi 14 Mai 2016 01:20:23
> Objet: RFR: 8144062: Move jdk.Version to java.lang.Runtime.Version
> 
> Hi.
> 
> Reviving this work from a few months back.
> 
> Please review the following changes to move jdk.Version to
> java.lang.Runtime.Version.
> 
> Bug
> 
> 8144062: Move jdk.Version to java.lang.Runtime.Version
> https://bugs.openjdk.java.net/browse/JDK-8144062
> 
> webrev
> 
> http://cr.openjdk.java.net/~iris/verona/8144062/webrev.1/
> 
> When jdk.Version was initially pushed in jdk-9+1-5, it was
> Improperly exported by java.base.  After exploring a few
> options, the best choice was to move jdk.Version to
> java.lang.Runtime.Version (a nested class of Runtime).  By
> making Version an SE API, it may be exported by the java.base
> module.
> 
> As part of the move, a limited number of chnages were
> made to the Version class:
> 
>   - Change package name and class declaration (to static)
>   - Eliminate use of "JDK" when describing a Java SE API
>   - Initial clarifications related to zeros (trailing vs.
> Internal components)
>   - Small typographical and grammatical enhancements
>   - Indentation
> 
> The complete Runtime.Version specification is available here:
> 
>   
> http://cr.openjdk.java.net/~iris/verona/8144062/doc.1/java/lang/Runtime.Version.html
> 
> The old jdk.Version.current() was replaced with
> Runtime.version().
> 
> In System.getProperties(), we indicate which version-related
> system properties may be supported by Runtime.Version.
> 
> The remaining jdk and langtools file changes are all
> side-effects of changing jdk.Version.current() to
> Runtime.version().
> 
> Thanks,
> Iris
>