Re: [OpenJDK 2D-Dev] [9] Review Request: 8042199 The build of J2DBench via makefile is broken after the JDK-8005402

2014-08-14 Thread Andrew Brygin

Hello Sergey,

 the change looks fine to me.

Thanks,
Andrew

On 8/13/2014 4:31 AM, Sergey Bylokhov wrote:

Hi Jim.
Yes, you are right, I missed it even after attentive viewing.
Typo was fixed:
http://cr.openjdk.java.net/~serb/8042199/webrev.03


Hi Sergey,

I understand that the type was changed for a reason, but the variable is
spelled Platfrom - which is not a word - and the same text appears in
the comment there.

The word intended there is, I believe, Platform...



On 8/12/14 4:20 PM, Sergey Bylokhov wrote:

Hi, Jim.
Actually a Boolean was changed to a boolean, to skip autoboxing.
http://cr.openjdk.java.net/~serb/8042199/webrev.02/src/share/demo/java2d/J2DBench/src/j2dbench/tests/cmm/CMMTests.java.sdiff.html


The new Readme explanation looks good.  Thanks for updating the new code
for pre-1.5.
I notice that one of the changes (in CMMTests) is to a line with a typo
(Platfrom instead of Platform both in the code and in the comment on the
same line), but fixing the typo might affect a lot of other lines...?

...jim

On 8/12/14 8:32 AM, Sergey Bylokhov wrote:

On 12.08.2014 1:34, Jim Graham wrote:

Hi Sergey,

Is the -g:none the result of #2 below?

This was changed to align with javac debug=flase...  in build
xml(typo was fixed as well).

Also, if I read the email trail correctly then source/target=1.6 is
only there because JDK 9 compiler doesn't let you request anything
earlier. The Readme doesn't mention this and it should.

done.

Also, I'm not sure why it says that it requires at least 1.5 instead
of 1.4 now as there is no mention of any code changes that don't work
on 1.4 any more - were there?  The only explanation I saw below was
the source/target specs allowed by the 9 compiler, not any results of
trying to compile it on 1.4 or 1.5...

The reason was in the some java features(@overried/enums) in the new
colors management tests from JDK-8005402. In the last version I fix
that, and now we can compile the tests using jdk 1.4.2. Note that 1.3 is
not supported, because the new tests uses some api which was added in 1.4.

The new version of the fix:
http://cr.openjdk.java.net/~serb/8042199/webrev.02


So, the Readme should minimally mention that source/target is set to
1.6 in the makefile only because of support in the 9 compiler, and we
should double check which compilers it actually is still buildable on
and record that in the Readme.  (Again, maybe I missed the part where
we tried it on 1.4 and failed, but it works on 1.5 - that wasn't
included below...)

  ...jim

On 8/11/14 9:01 AM, Sergey Bylokhov wrote:

Hello.
Please review the new version of the fix:
http://cr.openjdk.java.net/~serb/8042199/webrev.01
- targetsource changed to 1.6. But readme still mentions that
benchmark requires at least jdk1.5 to compile.
- I found mismatch between ant/make about debug information. fixed
- the fix for 8005402 did not properly update makefiles for images.
fixed
- dest was changed to dist, because this is default location of
J2DBench.jar.

On 07.08.2014 23:55, Jim Graham wrote:

The only intention was that we be able to compare against older
runtimes when needed.  We could ask whether we care about how we
currently compare against 1.2 any more...?  If we're really that
curious, we can either change the target and compile with an older
compiler, or find an older version of it (but that would be a little
apples-to-oranges).  In any case, we'd have options for doing it even
if they weren't as convenient as just running it on an older jvm.

It's convenience and need vs. what's possible and right now need
is probably a very small value (for 1.5) and what's possible just
changed...

  ...jim

On 8/7/14 11:31 AM, Phil Race wrote:

Perhaps we have to make that the default but add a comment that since
this
is bundled with JDK 9 it must use at least a 1.6 target but the
intention is
that it be able to be compiled with and targeted to, earlier JDKs

BTW I guess dest-dist is OK but I imagine Jim really did mean dest

-# java -jar dest/J2DBench.jar -batch -loadopts options/default.opt \
+# java -jar dist/J2DBench.jar -batch -loadopts options/default.opt \


-phil.

On 8/7/2014 9:23 AM, Sergey Bylokhov wrote:

Hello, Phil.
jdk 9 now supports -target 1.6+/-source 1.6+ options only. Looks
like we should use this minimum version too.
If you have no objections I'll prepare the new version of the fix

On 14.05.2014 21:09, Phil Race wrote:

Hmm .. the thing here is that I know that Jim was very keen that
this
benchmark always be runnable on JDK 1.2
Deleting the comment to that effect and setting source level to 1.5
goes against this.
What is the rationale, and why can't it be reverted to be able to
build on 1.4 and run
on 1.2 ? If it uses JDK 1.5 language features, just back them
out. If
it uses JDK 1.5
APIs then maybe Jim had to handle something similar and has an
idea ?

-phil.

On 4/30/2014 4:13 AM, Andrew Brygin wrote:

Hello Sergey,

   the change looks fine to 

Re: [OpenJDK 2D-Dev] [9] Review Request: 8042199 The build of J2DBench via makefile is broken after the JDK-8005402

2014-08-12 Thread Sergey Bylokhov

On 12.08.2014 1:34, Jim Graham wrote:

Hi Sergey,

Is the -g:none the result of #2 below?
This was changed to align with javac debug=flase...  in build 
xml(typo was fixed as well).


Also, if I read the email trail correctly then source/target=1.6 is 
only there because JDK 9 compiler doesn't let you request anything 
earlier. The Readme doesn't mention this and it should.

done.


Also, I'm not sure why it says that it requires at least 1.5 instead 
of 1.4 now as there is no mention of any code changes that don't work 
on 1.4 any more - were there?  The only explanation I saw below was 
the source/target specs allowed by the 9 compiler, not any results of 
trying to compile it on 1.4 or 1.5...
The reason was in the some java features(@overried/enums) in the new 
colors management tests from JDK-8005402. In the last version I fix 
that, and now we can compile the tests using jdk 1.4.2. Note that 1.3 is 
not supported, because the new tests uses some api which was added in 1.4.


The new version of the fix:
http://cr.openjdk.java.net/~serb/8042199/webrev.02



So, the Readme should minimally mention that source/target is set to 
1.6 in the makefile only because of support in the 9 compiler, and we 
should double check which compilers it actually is still buildable on 
and record that in the Readme.  (Again, maybe I missed the part where 
we tried it on 1.4 and failed, but it works on 1.5 - that wasn't 
included below...)


...jim

On 8/11/14 9:01 AM, Sergey Bylokhov wrote:

Hello.
Please review the new version of the fix:
http://cr.openjdk.java.net/~serb/8042199/webrev.01
  - targetsource changed to 1.6. But readme still mentions that
benchmark requires at least jdk1.5 to compile.
  - I found mismatch between ant/make about debug information. fixed
  - the fix for 8005402 did not properly update makefiles for images. 
fixed

  - dest was changed to dist, because this is default location of
J2DBench.jar.

On 07.08.2014 23:55, Jim Graham wrote:

The only intention was that we be able to compare against older
runtimes when needed.  We could ask whether we care about how we
currently compare against 1.2 any more...?  If we're really that
curious, we can either change the target and compile with an older
compiler, or find an older version of it (but that would be a little
apples-to-oranges).  In any case, we'd have options for doing it even
if they weren't as convenient as just running it on an older jvm.

It's convenience and need vs. what's possible and right now need
is probably a very small value (for 1.5) and what's possible just
changed...

...jim

On 8/7/14 11:31 AM, Phil Race wrote:

Perhaps we have to make that the default but add a comment that since
this
is bundled with JDK 9 it must use at least a 1.6 target but the
intention is
that it be able to be compiled with and targeted to, earlier JDKs

BTW I guess dest-dist is OK but I imagine Jim really did mean dest

-# java -jar dest/J2DBench.jar -batch -loadopts options/default.opt \
+# java -jar dist/J2DBench.jar -batch -loadopts options/default.opt \


-phil.

On 8/7/2014 9:23 AM, Sergey Bylokhov wrote:

Hello, Phil.
jdk 9 now supports -target 1.6+/-source 1.6+ options only. Looks
like we should use this minimum version too.
If you have no objections I'll prepare the new version of the fix

On 14.05.2014 21:09, Phil Race wrote:
Hmm .. the thing here is that I know that Jim was very keen that 
this

benchmark always be runnable on JDK 1.2
Deleting the comment to that effect and setting source level to 1.5
goes against this.
What is the rationale, and why can't it be reverted to be able to
build on 1.4 and run
on 1.2 ? If it uses JDK 1.5 language features, just back them 
out. If

it uses JDK 1.5
APIs then maybe Jim had to handle something similar and has an 
idea ?


-phil.

On 4/30/2014 4:13 AM, Andrew Brygin wrote:

Hello Sergey,

 the change looks fine to me.

Thanks,
Andrew

On 4/30/2014 3:12 PM, Sergey Bylokhov wrote:

Hello.
Please review the small fix for jdk 9.
Makefile and README were fixed.

Bug: https://bugs.openjdk.java.net/browse/JDK-8042199
Webrev can be found at:
http://cr.openjdk.java.net/~serb/8042199/webrev.00

--
Best regards, Sergey.















--
Best regards, Sergey.



Re: [OpenJDK 2D-Dev] [9] Review Request: 8042199 The build of J2DBench via makefile is broken after the JDK-8005402

2014-08-12 Thread Jim Graham
The new Readme explanation looks good.  Thanks for updating the new code 
for pre-1.5.


I notice that one of the changes (in CMMTests) is to a line with a typo 
(Platfrom instead of Platform both in the code and in the comment on the 
same line), but fixing the typo might affect a lot of other lines...?


...jim

On 8/12/14 8:32 AM, Sergey Bylokhov wrote:

On 12.08.2014 1:34, Jim Graham wrote:

Hi Sergey,

Is the -g:none the result of #2 below?

This was changed to align with javac debug=flase...  in build
xml(typo was fixed as well).


Also, if I read the email trail correctly then source/target=1.6 is
only there because JDK 9 compiler doesn't let you request anything
earlier. The Readme doesn't mention this and it should.

done.


Also, I'm not sure why it says that it requires at least 1.5 instead
of 1.4 now as there is no mention of any code changes that don't work
on 1.4 any more - were there?  The only explanation I saw below was
the source/target specs allowed by the 9 compiler, not any results of
trying to compile it on 1.4 or 1.5...

The reason was in the some java features(@overried/enums) in the new
colors management tests from JDK-8005402. In the last version I fix
that, and now we can compile the tests using jdk 1.4.2. Note that 1.3 is
not supported, because the new tests uses some api which was added in 1.4.

The new version of the fix:
http://cr.openjdk.java.net/~serb/8042199/webrev.02



So, the Readme should minimally mention that source/target is set to
1.6 in the makefile only because of support in the 9 compiler, and we
should double check which compilers it actually is still buildable on
and record that in the Readme.  (Again, maybe I missed the part where
we tried it on 1.4 and failed, but it works on 1.5 - that wasn't
included below...)

...jim

On 8/11/14 9:01 AM, Sergey Bylokhov wrote:

Hello.
Please review the new version of the fix:
http://cr.openjdk.java.net/~serb/8042199/webrev.01
  - targetsource changed to 1.6. But readme still mentions that
benchmark requires at least jdk1.5 to compile.
  - I found mismatch between ant/make about debug information. fixed
  - the fix for 8005402 did not properly update makefiles for images.
fixed
  - dest was changed to dist, because this is default location of
J2DBench.jar.

On 07.08.2014 23:55, Jim Graham wrote:

The only intention was that we be able to compare against older
runtimes when needed.  We could ask whether we care about how we
currently compare against 1.2 any more...?  If we're really that
curious, we can either change the target and compile with an older
compiler, or find an older version of it (but that would be a little
apples-to-oranges).  In any case, we'd have options for doing it even
if they weren't as convenient as just running it on an older jvm.

It's convenience and need vs. what's possible and right now need
is probably a very small value (for 1.5) and what's possible just
changed...

...jim

On 8/7/14 11:31 AM, Phil Race wrote:

Perhaps we have to make that the default but add a comment that since
this
is bundled with JDK 9 it must use at least a 1.6 target but the
intention is
that it be able to be compiled with and targeted to, earlier JDKs

BTW I guess dest-dist is OK but I imagine Jim really did mean dest

-# java -jar dest/J2DBench.jar -batch -loadopts options/default.opt \
+# java -jar dist/J2DBench.jar -batch -loadopts options/default.opt \


-phil.

On 8/7/2014 9:23 AM, Sergey Bylokhov wrote:

Hello, Phil.
jdk 9 now supports -target 1.6+/-source 1.6+ options only. Looks
like we should use this minimum version too.
If you have no objections I'll prepare the new version of the fix

On 14.05.2014 21:09, Phil Race wrote:

Hmm .. the thing here is that I know that Jim was very keen that
this
benchmark always be runnable on JDK 1.2
Deleting the comment to that effect and setting source level to 1.5
goes against this.
What is the rationale, and why can't it be reverted to be able to
build on 1.4 and run
on 1.2 ? If it uses JDK 1.5 language features, just back them
out. If
it uses JDK 1.5
APIs then maybe Jim had to handle something similar and has an
idea ?

-phil.

On 4/30/2014 4:13 AM, Andrew Brygin wrote:

Hello Sergey,

 the change looks fine to me.

Thanks,
Andrew

On 4/30/2014 3:12 PM, Sergey Bylokhov wrote:

Hello.
Please review the small fix for jdk 9.
Makefile and README were fixed.

Bug: https://bugs.openjdk.java.net/browse/JDK-8042199
Webrev can be found at:
http://cr.openjdk.java.net/~serb/8042199/webrev.00

--
Best regards, Sergey.

















Re: [OpenJDK 2D-Dev] [9] Review Request: 8042199 The build of J2DBench via makefile is broken after the JDK-8005402

2014-08-12 Thread Sergey Bylokhov
Hi, Jim.
Actually a Boolean was changed to a boolean, to skip autoboxing.
http://cr.openjdk.java.net/~serb/8042199/webrev.02/src/share/demo/java2d/J2DBench/src/j2dbench/tests/cmm/CMMTests.java.sdiff.html

 The new Readme explanation looks good.  Thanks for updating the new code 
 for pre-1.5.

 I notice that one of the changes (in CMMTests) is to a line with a typo 
 (Platfrom instead of Platform both in the code and in the comment on the 
 same line), but fixing the typo might affect a lot of other lines...?

...jim

On 8/12/14 8:32 AM, Sergey Bylokhov wrote:
 On 12.08.2014 1:34, Jim Graham wrote:
 Hi Sergey,

 Is the -g:none the result of #2 below?
 This was changed to align with javac debug=flase...  in build
 xml(typo was fixed as well).

 Also, if I read the email trail correctly then source/target=1.6 is
 only there because JDK 9 compiler doesn't let you request anything
 earlier. The Readme doesn't mention this and it should.
 done.

 Also, I'm not sure why it says that it requires at least 1.5 instead
 of 1.4 now as there is no mention of any code changes that don't work
 on 1.4 any more - were there?  The only explanation I saw below was
 the source/target specs allowed by the 9 compiler, not any results of
 trying to compile it on 1.4 or 1.5...
 The reason was in the some java features(@overried/enums) in the new
 colors management tests from JDK-8005402. In the last version I fix
 that, and now we can compile the tests using jdk 1.4.2. Note that 1.3 is
 not supported, because the new tests uses some api which was added in 1.4.

 The new version of the fix:
 http://cr.openjdk.java.net/~serb/8042199/webrev.02


 So, the Readme should minimally mention that source/target is set to
 1.6 in the makefile only because of support in the 9 compiler, and we
 should double check which compilers it actually is still buildable on
 and record that in the Readme.  (Again, maybe I missed the part where
 we tried it on 1.4 and failed, but it works on 1.5 - that wasn't
 included below...)

 ...jim

 On 8/11/14 9:01 AM, Sergey Bylokhov wrote:
 Hello.
 Please review the new version of the fix:
 http://cr.openjdk.java.net/~serb/8042199/webrev.01
   - targetsource changed to 1.6. But readme still mentions that
 benchmark requires at least jdk1.5 to compile.
   - I found mismatch between ant/make about debug information. fixed
   - the fix for 8005402 did not properly update makefiles for images.
 fixed
   - dest was changed to dist, because this is default location of
 J2DBench.jar.

 On 07.08.2014 23:55, Jim Graham wrote:
 The only intention was that we be able to compare against older
 runtimes when needed.  We could ask whether we care about how we
 currently compare against 1.2 any more...?  If we're really that
 curious, we can either change the target and compile with an older
 compiler, or find an older version of it (but that would be a little
 apples-to-oranges).  In any case, we'd have options for doing it even
 if they weren't as convenient as just running it on an older jvm.

 It's convenience and need vs. what's possible and right now need
 is probably a very small value (for 1.5) and what's possible just
 changed...

 ...jim

 On 8/7/14 11:31 AM, Phil Race wrote:
 Perhaps we have to make that the default but add a comment that since
 this
 is bundled with JDK 9 it must use at least a 1.6 target but the
 intention is
 that it be able to be compiled with and targeted to, earlier JDKs

 BTW I guess dest-dist is OK but I imagine Jim really did mean dest

 -# java -jar dest/J2DBench.jar -batch -loadopts options/default.opt \
 +# java -jar dist/J2DBench.jar -batch -loadopts options/default.opt \


 -phil.

 On 8/7/2014 9:23 AM, Sergey Bylokhov wrote:
 Hello, Phil.
 jdk 9 now supports -target 1.6+/-source 1.6+ options only. Looks
 like we should use this minimum version too.
 If you have no objections I'll prepare the new version of the fix

 On 14.05.2014 21:09, Phil Race wrote:
 Hmm .. the thing here is that I know that Jim was very keen that
 this
 benchmark always be runnable on JDK 1.2
 Deleting the comment to that effect and setting source level to 1.5
 goes against this.
 What is the rationale, and why can't it be reverted to be able to
 build on 1.4 and run
 on 1.2 ? If it uses JDK 1.5 language features, just back them
 out. If
 it uses JDK 1.5
 APIs then maybe Jim had to handle something similar and has an
 idea ?

 -phil.

 On 4/30/2014 4:13 AM, Andrew Brygin wrote:
 Hello Sergey,

  the change looks fine to me.

 Thanks,
 Andrew

 On 4/30/2014 3:12 PM, Sergey Bylokhov wrote:
 Hello.
 Please review the small fix for jdk 9.
 Makefile and README were fixed.

 Bug: https://bugs.openjdk.java.net/browse/JDK-8042199
 Webrev can be found at:
 http://cr.openjdk.java.net/~serb/8042199/webrev.00

 --
 Best regards, Sergey.











Re: [OpenJDK 2D-Dev] [9] Review Request: 8042199 The build of J2DBench via makefile is broken after the JDK-8005402

2014-08-12 Thread Jim Graham

Hi Sergey,

I understand that the type was changed for a reason, but the variable is 
spelled Platfrom - which is not a word - and the same text appears in 
the comment there.


The word intended there is, I believe, Platform...

...jim

On 8/12/14 4:20 PM, Sergey Bylokhov wrote:

Hi, Jim.
Actually a Boolean was changed to a boolean, to skip autoboxing.
http://cr.openjdk.java.net/~serb/8042199/webrev.02/src/share/demo/java2d/J2DBench/src/j2dbench/tests/cmm/CMMTests.java.sdiff.html


The new Readme explanation looks good.  Thanks for updating the new code
for pre-1.5.



I notice that one of the changes (in CMMTests) is to a line with a typo
(Platfrom instead of Platform both in the code and in the comment on the
same line), but fixing the typo might affect a lot of other lines...?


...jim

On 8/12/14 8:32 AM, Sergey Bylokhov wrote:

On 12.08.2014 1:34, Jim Graham wrote:

Hi Sergey,

Is the -g:none the result of #2 below?

This was changed to align with javac debug=flase...  in build
xml(typo was fixed as well).


Also, if I read the email trail correctly then source/target=1.6 is
only there because JDK 9 compiler doesn't let you request anything
earlier. The Readme doesn't mention this and it should.

done.


Also, I'm not sure why it says that it requires at least 1.5 instead
of 1.4 now as there is no mention of any code changes that don't work
on 1.4 any more - were there?  The only explanation I saw below was
the source/target specs allowed by the 9 compiler, not any results of
trying to compile it on 1.4 or 1.5...

The reason was in the some java features(@overried/enums) in the new
colors management tests from JDK-8005402. In the last version I fix
that, and now we can compile the tests using jdk 1.4.2. Note that 1.3 is
not supported, because the new tests uses some api which was added in 1.4.

The new version of the fix:
http://cr.openjdk.java.net/~serb/8042199/webrev.02



So, the Readme should minimally mention that source/target is set to
1.6 in the makefile only because of support in the 9 compiler, and we
should double check which compilers it actually is still buildable on
and record that in the Readme.  (Again, maybe I missed the part where
we tried it on 1.4 and failed, but it works on 1.5 - that wasn't
included below...)

 ...jim

On 8/11/14 9:01 AM, Sergey Bylokhov wrote:

Hello.
Please review the new version of the fix:
http://cr.openjdk.java.net/~serb/8042199/webrev.01
   - targetsource changed to 1.6. But readme still mentions that
benchmark requires at least jdk1.5 to compile.
   - I found mismatch between ant/make about debug information. fixed
   - the fix for 8005402 did not properly update makefiles for images.
fixed
   - dest was changed to dist, because this is default location of
J2DBench.jar.

On 07.08.2014 23:55, Jim Graham wrote:

The only intention was that we be able to compare against older
runtimes when needed.  We could ask whether we care about how we
currently compare against 1.2 any more...?  If we're really that
curious, we can either change the target and compile with an older
compiler, or find an older version of it (but that would be a little
apples-to-oranges).  In any case, we'd have options for doing it even
if they weren't as convenient as just running it on an older jvm.

It's convenience and need vs. what's possible and right now need
is probably a very small value (for 1.5) and what's possible just
changed...

 ...jim

On 8/7/14 11:31 AM, Phil Race wrote:

Perhaps we have to make that the default but add a comment that since
this
is bundled with JDK 9 it must use at least a 1.6 target but the
intention is
that it be able to be compiled with and targeted to, earlier JDKs

BTW I guess dest-dist is OK but I imagine Jim really did mean dest

-# java -jar dest/J2DBench.jar -batch -loadopts options/default.opt \
+# java -jar dist/J2DBench.jar -batch -loadopts options/default.opt \


-phil.

On 8/7/2014 9:23 AM, Sergey Bylokhov wrote:

Hello, Phil.
jdk 9 now supports -target 1.6+/-source 1.6+ options only. Looks
like we should use this minimum version too.
If you have no objections I'll prepare the new version of the fix

On 14.05.2014 21:09, Phil Race wrote:

Hmm .. the thing here is that I know that Jim was very keen that
this
benchmark always be runnable on JDK 1.2
Deleting the comment to that effect and setting source level to 1.5
goes against this.
What is the rationale, and why can't it be reverted to be able to
build on 1.4 and run
on 1.2 ? If it uses JDK 1.5 language features, just back them
out. If
it uses JDK 1.5
APIs then maybe Jim had to handle something similar and has an
idea ?

-phil.

On 4/30/2014 4:13 AM, Andrew Brygin wrote:

Hello Sergey,

  the change looks fine to me.

Thanks,
Andrew

On 4/30/2014 3:12 PM, Sergey Bylokhov wrote:

Hello.
Please review the small fix for jdk 9.
Makefile and README were fixed.

Bug: https://bugs.openjdk.java.net/browse/JDK-8042199
Webrev can be 

Re: [OpenJDK 2D-Dev] [9] Review Request: 8042199 The build of J2DBench via makefile is broken after the JDK-8005402

2014-08-12 Thread Sergey Bylokhov
Hi Jim.
Yes, you are right, I missed it even after attentive viewing.
Typo was fixed:
http://cr.openjdk.java.net/~serb/8042199/webrev.03

 Hi Sergey,
 
 I understand that the type was changed for a reason, but the variable is 
 spelled Platfrom - which is not a word - and the same text appears in 
 the comment there.
 
 The word intended there is, I believe, Platform...



On 8/12/14 4:20 PM, Sergey Bylokhov wrote:
 Hi, Jim.
 Actually a Boolean was changed to a boolean, to skip autoboxing.
 http://cr.openjdk.java.net/~serb/8042199/webrev.02/src/share/demo/java2d/J2DBench/src/j2dbench/tests/cmm/CMMTests.java.sdiff.html

 The new Readme explanation looks good.  Thanks for updating the new code
 for pre-1.5.

 I notice that one of the changes (in CMMTests) is to a line with a typo
 (Platfrom instead of Platform both in the code and in the comment on the
 same line), but fixing the typo might affect a lot of other lines...?

   ...jim

 On 8/12/14 8:32 AM, Sergey Bylokhov wrote:
 On 12.08.2014 1:34, Jim Graham wrote:
 Hi Sergey,

 Is the -g:none the result of #2 below?
 This was changed to align with javac debug=flase...  in build
 xml(typo was fixed as well).

 Also, if I read the email trail correctly then source/target=1.6 is
 only there because JDK 9 compiler doesn't let you request anything
 earlier. The Readme doesn't mention this and it should.
 done.

 Also, I'm not sure why it says that it requires at least 1.5 instead
 of 1.4 now as there is no mention of any code changes that don't work
 on 1.4 any more - were there?  The only explanation I saw below was
 the source/target specs allowed by the 9 compiler, not any results of
 trying to compile it on 1.4 or 1.5...
 The reason was in the some java features(@overried/enums) in the new
 colors management tests from JDK-8005402. In the last version I fix
 that, and now we can compile the tests using jdk 1.4.2. Note that 1.3 is
 not supported, because the new tests uses some api which was added in 1.4.

 The new version of the fix:
 http://cr.openjdk.java.net/~serb/8042199/webrev.02


 So, the Readme should minimally mention that source/target is set to
 1.6 in the makefile only because of support in the 9 compiler, and we
 should double check which compilers it actually is still buildable on
 and record that in the Readme.  (Again, maybe I missed the part where
 we tried it on 1.4 and failed, but it works on 1.5 - that wasn't
 included below...)

  ...jim

 On 8/11/14 9:01 AM, Sergey Bylokhov wrote:
 Hello.
 Please review the new version of the fix:
 http://cr.openjdk.java.net/~serb/8042199/webrev.01
- targetsource changed to 1.6. But readme still mentions that
 benchmark requires at least jdk1.5 to compile.
- I found mismatch between ant/make about debug information. fixed
- the fix for 8005402 did not properly update makefiles for images.
 fixed
- dest was changed to dist, because this is default location of
 J2DBench.jar.

 On 07.08.2014 23:55, Jim Graham wrote:
 The only intention was that we be able to compare against older
 runtimes when needed.  We could ask whether we care about how we
 currently compare against 1.2 any more...?  If we're really that
 curious, we can either change the target and compile with an older
 compiler, or find an older version of it (but that would be a little
 apples-to-oranges).  In any case, we'd have options for doing it even
 if they weren't as convenient as just running it on an older jvm.

 It's convenience and need vs. what's possible and right now need
 is probably a very small value (for 1.5) and what's possible just
 changed...

  ...jim

 On 8/7/14 11:31 AM, Phil Race wrote:
 Perhaps we have to make that the default but add a comment that since
 this
 is bundled with JDK 9 it must use at least a 1.6 target but the
 intention is
 that it be able to be compiled with and targeted to, earlier JDKs

 BTW I guess dest-dist is OK but I imagine Jim really did mean dest

 -# java -jar dest/J2DBench.jar -batch -loadopts options/default.opt \
 +# java -jar dist/J2DBench.jar -batch -loadopts options/default.opt \


 -phil.

 On 8/7/2014 9:23 AM, Sergey Bylokhov wrote:
 Hello, Phil.
 jdk 9 now supports -target 1.6+/-source 1.6+ options only. Looks
 like we should use this minimum version too.
 If you have no objections I'll prepare the new version of the fix

 On 14.05.2014 21:09, Phil Race wrote:
 Hmm .. the thing here is that I know that Jim was very keen that
 this
 benchmark always be runnable on JDK 1.2
 Deleting the comment to that effect and setting source level to 1.5
 goes against this.
 What is the rationale, and why can't it be reverted to be able to
 build on 1.4 and run
 on 1.2 ? If it uses JDK 1.5 language features, just back them
 out. If
 it uses JDK 1.5
 APIs then maybe Jim had to handle something similar and has an
 idea ?

 -phil.

 On 4/30/2014 4:13 AM, Andrew Brygin wrote:
 Hello Sergey,

   the change looks fine to me.

 Thanks,
 Andrew

 On 

Re: [OpenJDK 2D-Dev] [9] Review Request: 8042199 The build of J2DBench via makefile is broken after the JDK-8005402

2014-08-12 Thread Jim Graham

Thanks Sergey, looks good - approved.

It's interesting to note that you fixed the text that controls the 
name of the command line option and option listed in results files, but 
since the option is commented out anyway then I'm guessing that there 
are no benchmark scripts that could have been relying on it anyway.  If 
there are, then they would need to be updated if we ever enable this 
switch again, but that's an internal issue...


...jim

On 8/12/14 5:31 PM, Sergey Bylokhov wrote:

Hi Jim.
Yes, you are right, I missed it even after attentive viewing.
Typo was fixed:
http://cr.openjdk.java.net/~serb/8042199/webrev.03


Hi Sergey,

I understand that the type was changed for a reason, but the variable is
spelled Platfrom - which is not a word - and the same text appears in
the comment there.

The word intended there is, I believe, Platform...




On 8/12/14 4:20 PM, Sergey Bylokhov wrote:

Hi, Jim.
Actually a Boolean was changed to a boolean, to skip autoboxing.
http://cr.openjdk.java.net/~serb/8042199/webrev.02/src/share/demo/java2d/J2DBench/src/j2dbench/tests/cmm/CMMTests.java.sdiff.html


The new Readme explanation looks good.  Thanks for updating the new code
for pre-1.5.



I notice that one of the changes (in CMMTests) is to a line with a typo
(Platfrom instead of Platform both in the code and in the comment on the
same line), but fixing the typo might affect a lot of other lines...?


...jim

On 8/12/14 8:32 AM, Sergey Bylokhov wrote:

On 12.08.2014 1:34, Jim Graham wrote:

Hi Sergey,

Is the -g:none the result of #2 below?

This was changed to align with javac debug=flase...  in build
xml(typo was fixed as well).


Also, if I read the email trail correctly then source/target=1.6 is
only there because JDK 9 compiler doesn't let you request anything
earlier. The Readme doesn't mention this and it should.

done.


Also, I'm not sure why it says that it requires at least 1.5 instead
of 1.4 now as there is no mention of any code changes that don't work
on 1.4 any more - were there?  The only explanation I saw below was
the source/target specs allowed by the 9 compiler, not any results of
trying to compile it on 1.4 or 1.5...

The reason was in the some java features(@overried/enums) in the new
colors management tests from JDK-8005402. In the last version I fix
that, and now we can compile the tests using jdk 1.4.2. Note that 1.3 is
not supported, because the new tests uses some api which was added in 1.4.

The new version of the fix:
http://cr.openjdk.java.net/~serb/8042199/webrev.02



So, the Readme should minimally mention that source/target is set to
1.6 in the makefile only because of support in the 9 compiler, and we
should double check which compilers it actually is still buildable on
and record that in the Readme.  (Again, maybe I missed the part where
we tried it on 1.4 and failed, but it works on 1.5 - that wasn't
included below...)

  ...jim

On 8/11/14 9:01 AM, Sergey Bylokhov wrote:

Hello.
Please review the new version of the fix:
http://cr.openjdk.java.net/~serb/8042199/webrev.01
- targetsource changed to 1.6. But readme still mentions that
benchmark requires at least jdk1.5 to compile.
- I found mismatch between ant/make about debug information. fixed
- the fix for 8005402 did not properly update makefiles for images.
fixed
- dest was changed to dist, because this is default location of
J2DBench.jar.

On 07.08.2014 23:55, Jim Graham wrote:

The only intention was that we be able to compare against older
runtimes when needed.  We could ask whether we care about how we
currently compare against 1.2 any more...?  If we're really that
curious, we can either change the target and compile with an older
compiler, or find an older version of it (but that would be a little
apples-to-oranges).  In any case, we'd have options for doing it even
if they weren't as convenient as just running it on an older jvm.

It's convenience and need vs. what's possible and right now need
is probably a very small value (for 1.5) and what's possible just
changed...

  ...jim

On 8/7/14 11:31 AM, Phil Race wrote:

Perhaps we have to make that the default but add a comment that since
this
is bundled with JDK 9 it must use at least a 1.6 target but the
intention is
that it be able to be compiled with and targeted to, earlier JDKs

BTW I guess dest-dist is OK but I imagine Jim really did mean dest

-# java -jar dest/J2DBench.jar -batch -loadopts options/default.opt \
+# java -jar dist/J2DBench.jar -batch -loadopts options/default.opt \


-phil.

On 8/7/2014 9:23 AM, Sergey Bylokhov wrote:

Hello, Phil.
jdk 9 now supports -target 1.6+/-source 1.6+ options only. Looks
like we should use this minimum version too.
If you have no objections I'll prepare the new version of the fix

On 14.05.2014 21:09, Phil Race wrote:

Hmm .. the thing here is that I know that Jim was very keen that
this
benchmark always be runnable on JDK 1.2
Deleting 

Re: [OpenJDK 2D-Dev] [9] Review Request: 8042199 The build of J2DBench via makefile is broken after the JDK-8005402

2014-08-11 Thread Jim Graham

Hi Sergey,

Is the -g:none the result of #2 below?

Also, if I read the email trail correctly then source/target=1.6 is only 
there because JDK 9 compiler doesn't let you request anything earlier. 
The Readme doesn't mention this and it should.


Also, I'm not sure why it says that it requires at least 1.5 instead of 
1.4 now as there is no mention of any code changes that don't work on 
1.4 any more - were there?  The only explanation I saw below was the 
source/target specs allowed by the 9 compiler, not any results of trying 
to compile it on 1.4 or 1.5...


So, the Readme should minimally mention that source/target is set to 1.6 
in the makefile only because of support in the 9 compiler, and we should 
double check which compilers it actually is still buildable on and 
record that in the Readme.  (Again, maybe I missed the part where we 
tried it on 1.4 and failed, but it works on 1.5 - that wasn't included 
below...)


...jim

On 8/11/14 9:01 AM, Sergey Bylokhov wrote:

Hello.
Please review the new version of the fix:
http://cr.openjdk.java.net/~serb/8042199/webrev.01
  - targetsource changed to 1.6. But readme still mentions that
benchmark requires at least jdk1.5 to compile.
  - I found mismatch between ant/make about debug information. fixed
  - the fix for 8005402 did not properly update makefiles for images. fixed
  - dest was changed to dist, because this is default location of
J2DBench.jar.

On 07.08.2014 23:55, Jim Graham wrote:

The only intention was that we be able to compare against older
runtimes when needed.  We could ask whether we care about how we
currently compare against 1.2 any more...?  If we're really that
curious, we can either change the target and compile with an older
compiler, or find an older version of it (but that would be a little
apples-to-oranges).  In any case, we'd have options for doing it even
if they weren't as convenient as just running it on an older jvm.

It's convenience and need vs. what's possible and right now need
is probably a very small value (for 1.5) and what's possible just
changed...

...jim

On 8/7/14 11:31 AM, Phil Race wrote:

Perhaps we have to make that the default but add a comment that since
this
is bundled with JDK 9 it must use at least a 1.6 target but the
intention is
that it be able to be compiled with and targeted to, earlier JDKs

BTW I guess dest-dist is OK but I imagine Jim really did mean dest

-# java -jar dest/J2DBench.jar -batch -loadopts options/default.opt \
+# java -jar dist/J2DBench.jar -batch -loadopts options/default.opt \


-phil.

On 8/7/2014 9:23 AM, Sergey Bylokhov wrote:

Hello, Phil.
jdk 9 now supports -target 1.6+/-source 1.6+ options only. Looks
like we should use this minimum version too.
If you have no objections I'll prepare the new version of the fix

On 14.05.2014 21:09, Phil Race wrote:

Hmm .. the thing here is that I know that Jim was very keen that this
benchmark always be runnable on JDK 1.2
Deleting the comment to that effect and setting source level to 1.5
goes against this.
What is the rationale, and why can't it be reverted to be able to
build on 1.4 and run
on 1.2 ? If it uses JDK 1.5 language features, just back them out. If
it uses JDK 1.5
APIs then maybe Jim had to handle something similar and has an idea ?

-phil.

On 4/30/2014 4:13 AM, Andrew Brygin wrote:

Hello Sergey,

 the change looks fine to me.

Thanks,
Andrew

On 4/30/2014 3:12 PM, Sergey Bylokhov wrote:

Hello.
Please review the small fix for jdk 9.
Makefile and README were fixed.

Bug: https://bugs.openjdk.java.net/browse/JDK-8042199
Webrev can be found at:
http://cr.openjdk.java.net/~serb/8042199/webrev.00

--
Best regards, Sergey.














Re: [OpenJDK 2D-Dev] [9] Review Request: 8042199 The build of J2DBench via makefile is broken after the JDK-8005402

2014-08-07 Thread Sergey Bylokhov

Hello, Phil.
jdk 9 now supports -target 1.6+/-source 1.6+ options only. Looks like 
we should use this minimum version too.

If you have no objections I'll prepare the new version of the fix

On 14.05.2014 21:09, Phil Race wrote:
Hmm .. the thing here is that I know that Jim was very keen that this 
benchmark always be runnable on JDK 1.2
Deleting the comment to that effect and setting source level to 1.5 
goes against this.
What is the rationale, and why can't it be reverted to be able to 
build on 1.4 and run
on 1.2 ? If it uses JDK 1.5 language features, just back them out. If 
it uses JDK 1.5

APIs then maybe Jim had to handle something similar and has an idea ?

-phil.

On 4/30/2014 4:13 AM, Andrew Brygin wrote:

Hello Sergey,

 the change looks fine to me.

Thanks,
Andrew

On 4/30/2014 3:12 PM, Sergey Bylokhov wrote:

Hello.
Please review the small fix for jdk 9.
Makefile and README were fixed.

Bug: https://bugs.openjdk.java.net/browse/JDK-8042199
Webrev can be found at: 
http://cr.openjdk.java.net/~serb/8042199/webrev.00


--
Best regards, Sergey.







--
Best regards, Sergey.



Re: [OpenJDK 2D-Dev] [9] Review Request: 8042199 The build of J2DBench via makefile is broken after the JDK-8005402

2014-08-07 Thread Phil Race

Perhaps we have to make that the default but add a comment that since this
is bundled with JDK 9 it must use at least a 1.6 target but the intention is
that it be able to be compiled with and targeted to, earlier JDKs

BTW I guess dest-dist is OK but I imagine Jim really did mean dest

-# java -jar dest/J2DBench.jar -batch -loadopts options/default.opt \
+# java -jar dist/J2DBench.jar -batch -loadopts options/default.opt \


-phil.

On 8/7/2014 9:23 AM, Sergey Bylokhov wrote:

Hello, Phil.
jdk 9 now supports -target 1.6+/-source 1.6+ options only. Looks 
like we should use this minimum version too.

If you have no objections I'll prepare the new version of the fix

On 14.05.2014 21:09, Phil Race wrote:
Hmm .. the thing here is that I know that Jim was very keen that this 
benchmark always be runnable on JDK 1.2
Deleting the comment to that effect and setting source level to 1.5 
goes against this.
What is the rationale, and why can't it be reverted to be able to 
build on 1.4 and run
on 1.2 ? If it uses JDK 1.5 language features, just back them out. If 
it uses JDK 1.5

APIs then maybe Jim had to handle something similar and has an idea ?

-phil.

On 4/30/2014 4:13 AM, Andrew Brygin wrote:

Hello Sergey,

 the change looks fine to me.

Thanks,
Andrew

On 4/30/2014 3:12 PM, Sergey Bylokhov wrote:

Hello.
Please review the small fix for jdk 9.
Makefile and README were fixed.

Bug: https://bugs.openjdk.java.net/browse/JDK-8042199
Webrev can be found at: 
http://cr.openjdk.java.net/~serb/8042199/webrev.00


--
Best regards, Sergey.











Re: [OpenJDK 2D-Dev] [9] Review Request: 8042199 The build of J2DBench via makefile is broken after the JDK-8005402

2014-08-07 Thread Jim Graham
The only intention was that we be able to compare against older runtimes 
when needed.  We could ask whether we care about how we currently 
compare against 1.2 any more...?  If we're really that curious, we can 
either change the target and compile with an older compiler, or find an 
older version of it (but that would be a little apples-to-oranges).  In 
any case, we'd have options for doing it even if they weren't as 
convenient as just running it on an older jvm.


It's convenience and need vs. what's possible and right now need 
is probably a very small value (for 1.5) and what's possible just 
changed...


...jim

On 8/7/14 11:31 AM, Phil Race wrote:

Perhaps we have to make that the default but add a comment that since this
is bundled with JDK 9 it must use at least a 1.6 target but the
intention is
that it be able to be compiled with and targeted to, earlier JDKs

BTW I guess dest-dist is OK but I imagine Jim really did mean dest

-# java -jar dest/J2DBench.jar -batch -loadopts options/default.opt \
+# java -jar dist/J2DBench.jar -batch -loadopts options/default.opt \


-phil.

On 8/7/2014 9:23 AM, Sergey Bylokhov wrote:

Hello, Phil.
jdk 9 now supports -target 1.6+/-source 1.6+ options only. Looks
like we should use this minimum version too.
If you have no objections I'll prepare the new version of the fix

On 14.05.2014 21:09, Phil Race wrote:

Hmm .. the thing here is that I know that Jim was very keen that this
benchmark always be runnable on JDK 1.2
Deleting the comment to that effect and setting source level to 1.5
goes against this.
What is the rationale, and why can't it be reverted to be able to
build on 1.4 and run
on 1.2 ? If it uses JDK 1.5 language features, just back them out. If
it uses JDK 1.5
APIs then maybe Jim had to handle something similar and has an idea ?

-phil.

On 4/30/2014 4:13 AM, Andrew Brygin wrote:

Hello Sergey,

 the change looks fine to me.

Thanks,
Andrew

On 4/30/2014 3:12 PM, Sergey Bylokhov wrote:

Hello.
Please review the small fix for jdk 9.
Makefile and README were fixed.

Bug: https://bugs.openjdk.java.net/browse/JDK-8042199
Webrev can be found at:
http://cr.openjdk.java.net/~serb/8042199/webrev.00

--
Best regards, Sergey.











Re: [OpenJDK 2D-Dev] [9] Review Request: 8042199 The build of J2DBench via makefile is broken after the JDK-8005402

2014-05-14 Thread Phil Race
Hmm .. the thing here is that I know that Jim was very keen that this 
benchmark always be runnable on JDK 1.2
Deleting the comment to that effect and setting source level to 1.5 goes 
against this.
What is the rationale, and why can't it be reverted to be able to build 
on 1.4 and run
on 1.2 ? If it uses JDK 1.5 language features, just back them out. If it 
uses JDK 1.5

APIs then maybe Jim had to handle something similar and has an idea ?

-phil.

On 4/30/2014 4:13 AM, Andrew Brygin wrote:

Hello Sergey,

 the change looks fine to me.

Thanks,
Andrew

On 4/30/2014 3:12 PM, Sergey Bylokhov wrote:

Hello.
Please review the small fix for jdk 9.
Makefile and README were fixed.

Bug: https://bugs.openjdk.java.net/browse/JDK-8042199
Webrev can be found at: 
http://cr.openjdk.java.net/~serb/8042199/webrev.00


--
Best regards, Sergey.