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

Tue Aug 12 23:20:38 UTC 2014

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.
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>>
>>>>>
>>>
>>>
>
>