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

Wed Aug 13 00:31:55 UTC 2014

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
>>>>    - target&source 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.
>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>
>>>>
>>>>
>>
>>