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
