On 5/7/2014 12:26 PM, Mandy Chung wrote:
On 5/7/2014 11:26 AM, Phil Race wrote:
https://bugs.openjdk.java.net/browse/JDK-8038875
http://cr.openjdk.java.net/~prr/8038875/
Thanks for taking this on. I have some minor comments:
line 129: if sun.java2d.renderer is set, it finds a custom
RenderingEngine from the bootclasspath (as the current class loader is
the bootstrap class loader). On the other hand, in the original
implementation, the service loader loads from the extension class
loader. I'm not proposing to use a different class loader since this
is not a supported way to plugin a custom rendering engine. Just to
mention it to call out the difference.
You may consider to use the 3-arg version of Class.forName and not to
initialize the class. This is called within a doPrivileged block and
it's generally a good convention to invoke the class initialization of
the specified class (from the system property) outside the
doPrivileged block. Static analysis tool may consider catching the
1-arg version of Class.forName and flag it as a warning.
The class will need to be initialized in the newInstance() call. The
above comment might not apply. Is the doPrivileged block needed for
the RenderingEngine instantiation? I was wondering if it was needed for
the service loader only or not.
Mandy
line 134-140: PiscesRenderingEngine is always present, right? Would it
be better just to return "new PiscesRenderingEngine()"?
otherwise, looks good.
Mandy
This is in support of modularisation (ie jigsaw). After the fix
alternate implementations
are loaded using Class.forName() as specified by the system property
sun.java2d.renderer. Note that only the built-in ones are actually
supported
The mechanism is just for testing.
BTW since it happens that Oracle JDK includes the pisces classes as well
as the closed source ductus I note that its possible to compare the
performance
or other characteristics easily in a closed build which has both by
specifying
to use pisces instead of ductus.
There's no regression test associated with this. I tested open &
closed builds
(created for all platforms) with standard demos.
-phil.
