[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-2d-dev
Subject:    [OpenJDK 2D-Dev] Review Reqeust for Bug 100068 - SunGraphics2D exposes a reference to itself while n
From:       mario.torre () aicas ! com (Mario Torre)
Date:       2009-08-05 8:37:09
Message-ID: 4A7944B5.8010106 () aicas ! com
[Download RAW message or body]

Il 04/08/2009 23:50, Jim Graham ha scritto:
> Roman Kennke wrote:
>>> http://cr.openjdk.java.net/~neugens/100068/webrev.06/
>>
>> So the short story is the webrev.05 was actually better and we better
>> forget about webrev.06 at this point?
>
> It also looks like the webrev.05 is better than a stock JDK - even more
> promising!

:)

Maybe someone with more experience could write a more "dramatic" test 
case, but yeah, I had the feeling that they are quite similar in the 
worst case, I find interesting that in 5 run each, sometimes the numbers 
are quite different. I assume this is a Linux problem, it would be 
interesting to run this test with a realtime VM on a realtime system :)

> It's only 5x instanceof in a single place in the code and it makes the
> entire business of loop validation much cleaner so I'm loving it in a
> global/general sense even if it is uglier at just that one line of code.

To be honest, it would be nice to have annotation do the same as marker 
interface, and being able to use them directly with instanceof without 
reflection, that would make everything a bit nicer, but this is really a 
minor cosmetic change, and doesn't change the ugly instanceof lines. I 
passed some time on it to find a better way, but could not think of any 
without the risk adding some overhead, so I gave up.

> However, I would modify the code style for it to move the curly to the
> following line like this:
>
> if (foopipe instanceof blah ||
> blahpipe instanceof blah ||
> barpipe instanceof blah)
> {
> sg2d.loops = ...;
> }
>
> This is almost completely in line with our code style guidelines (are
> those published on the OpenJDK site?) with the only minor variation
> being the open curly on the line by itself which is a personal
> preference (that is used through most of java2d) to make the break
> between multi-line conditionals and body more visible. I(we?) find that
> otherwise the body looks like another line of conditional tests...

Sure, that's fine! I was unsure about the coding guidelines, but this 
makes entirely sense to me.

I'll send you an update shortly based on the latest OpenJDK checkout even.

Cheers,
Mario
-- 
Mario Torre, Software Developer, http://www.jroller.com/neugens/
aicas Allerton Interworks Computer Automated Systems GmbH
Haid-und-Neu-Stra?e 18 * D-76131 Karlsruhe * Germany
http://www.aicas.com   * Tel: +49-721-663 968-44
pgp key: http://subkeys.pgp.net/ PGP Key ID: 80F240CF
Fingerprint: BA39 9666 94EC 8B73 27FA  FC7C 4086 63E3 80F2 40CF

USt-Id: DE216375633, Handelsregister HRB 109481, AG Mannheim
Gesch?ftsf?hrer: Dr. James J. Hunt

Please, support open standards:
http://endsoftpatents.org/



[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic