[OpenJDK 2D-Dev] Review Reqeust for Bug 100068 - SunGraphics2D exposes a reference to itself while non fully initialised

[Jim Graham]

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!

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

			...jim