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