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

Jim Graham james.graham at oracle.com
Wed Aug 13 00:50:45 UTC 2014


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



More information about the 2d-dev mailing list