[OpenJDK 2D-Dev] Review Reqeust for Bug 100068 - SunGraphics2D exposes a reference to itself while non fully initialised
Jim.A.Graham at Sun.COM
Tue Aug 4 21:50:51 UTC 2009
Roman Kennke wrote:
> 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
> Regarding webrev.05, I find the 5x instanceof a bit ugly. I am not sure
> how to avoid it though. Maybe put the pipe fields in a container. This
> container could have a (marker) subclass that is instanceof'ed for.
> Whenever a SD sets up loop based pipes, it uses the subclass. Otherwise
> it uses the base class. Then you'd only need one instanceof check
> against the container. But OTOH you would get double field access in a
> couple of cases, of which I don't know if hotspot optimizes them away
> somehow. And it's probably not worth thinking about any of this if
> impact is not noticable already. Maybe the if cascade bails out at the
> first check already? Maybe it's worth ordering the if cascade so that
> the most likely case is the first one, etc?
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.
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...
More information about the 2d-dev