[prev in list] [next in list] [prev in thread] [next in thread]
List: openjdk-2d-dev
Subject: Re: [OpenJDK 2D-Dev] [9] Review Request: 8042199 The build of J2DBench via makefile is broken after
From: Andrew Brygin <andrew.brygin () oracle ! com>
Date: 2014-08-14 14:28:57
Message-ID: 53ECC7A9.2090508 () oracle ! com
[Download RAW message or body]
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
> > > > > - 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.
> > > > > > > >
> > > > >
> > >
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic