On 10/21/2014 01:03 PM, Stanimir Simeonoff wrote:
I like the changes, esp the black listing which is long overdue in JDK
(middleware containers have been doing that for over a decade).
Not sure if there is any specification governing if two consecutive
attempts to load the same class name may respectively fail and succeed - if
the black listing passes, I believe that should go to classloading spec.
I don't think this is black listing what I do here. My only assumptions
are the following:
- if loadClass(name, resolve) has not been called for a particular name
before, then findLoadedClass(name) returns null
- if all calls to (parent.loadClass(name, resolve) /
findBootstrapClassOrNull()) and findClass(name) for particular name have
thrown ClassNotFoundException or returned null, then
findLoadedClass(name) returns null
That does not preclude the possibility of "transiently" failing and then
suddenly succeeding in loading a particular class. It only avoids
calling findLoadedClass(name) in situations when it would always return
null - and that are all 1st attempts at successful loading of a
particular class - the majority of requests.
Adding the initiating classloader in the Thread doesn't pose security risks
the way it's implemented, so it's fine imo.
As long as it's used only for optimization, I guess not. But if
Thread.initiatingClassLoader could be trusted, then we could have a
reference to the Class object in each Lock object and entirely eliminate
the need for calling findLoadedClass() in parallel capable class-loaders.
Regards, Peter
Stanimir
On Tue, Oct 21, 2014 at 1:37 PM, Peter Levart <[email protected]>
wrote:
Hi,
I guess we are not happy with stack-traces that only reveal call stack up
to the public ClassLoader.loadClass() method and hide everything happening
after that even if paired with a more descriptive message. So I improved
the strategy of throwing stack-less exception. New strategy throws
full-stack exception from findClass() if findClass() is invoked on the
"initiating" class loader and stack-less exception otherwise. How can we
know if a particular ClassLoader instance is the initiating class loader
for a particular class loading request? VM knows that when invoking
Class.forName() or when VM triggers class loading, because such calls are
wired through VM or initiate from VM. On the Java side, we can establish a
similar thread-local context at the entry point of Class.loadClass() public
method.
This strategy gives normal stack traces for exceptions that escape the
ClassLoader.loadClass() public method and avoids creating stack-full
exceptions that are later swallowed.
That's not all. I have another optimization at hand. Parallel capable
class loaders maintain a ConcurrentHashMap of Objects used as locks to
synchronize on when executing class loading request for a particular class
name. These seem like a waste of resource and can be used for more than
just locks. My 1st attempt at (ab)using these objects is the following:
http://cr.openjdk.java.net/~plevart/jdk9-dev/ClassLoader.CNFE/webrev.03/
Here a boolean flag is added to such Lock object that indicates whether a
class loading attempt has already been performed for a particular class
name. The 1st attempt at loading a particular class can therefore skip
calling findLoadedClass() native method. This has a measurable performance
impact as indicated in the following benchmark:
http://cr.openjdk.java.net/~plevart/jdk9-dev/ClassLoader.
CNFE/CLBench2.java
This is a variant of previous benchmark - only the part that measures
successful attempts at loading a class with a child of extension class
loader, but does that at different call-stack depths (10, 20, 40, 80, 160
frames). Original JDK 9 code gives these results:
Benchmark Mode Samples Score Error
Units
j.t.CLBench2.loadNewClassSuccessSS010 ss 5 3.947 ±
0.080 s
j.t.CLBench2.loadNewClassSuccessSS020 ss 5 4.001 ±
0.119 s
j.t.CLBench2.loadNewClassSuccessSS040 ss 5 4.068 ±
0.115 s
j.t.CLBench2.loadNewClassSuccessSS080 ss 5 4.323 ±
0.061 s
j.t.CLBench2.loadNewClassSuccessSS160 ss 5 4.633 ±
0.118 s
Doing stack-less CNF exceptions shows that call-stack depth does not have
an impact any more:
Benchmark Mode Samples Score Error
Units
j.t.CLBench2.loadNewClassSuccessSS010 ss 5 3.753 ±
0.155 s
j.t.CLBench2.loadNewClassSuccessSS020 ss 5 3.755 ±
0.160 s
j.t.CLBench2.loadNewClassSuccessSS040 ss 5 3.767 ±
0.124 s
j.t.CLBench2.loadNewClassSuccessSS080 ss 5 3.762 ±
0.148 s
j.t.CLBench2.loadNewClassSuccessSS160 ss 5 3.781 ±
0.146 s
Adding findLoadedClass() avoidance on top of that gives even better
results:
Benchmark Mode Samples Score Error
Units
j.t.CLBench2.loadNewClassSuccessSS010 ss 5 3.539 ±
0.185 s
j.t.CLBench2.loadNewClassSuccessSS020 ss 5 3.548 ±
0.165 s
j.t.CLBench2.loadNewClassSuccessSS040 ss 5 3.586 ±
0.226 s
j.t.CLBench2.loadNewClassSuccessSS080 ss 5 3.541 ±
0.053 s
j.t.CLBench2.loadNewClassSuccessSS160 ss 5 3.570 ±
0.113 s
Regards, Peter
On 10/16/2014 04:05 PM, Peter Levart wrote:
I created an issue in Jira to track this investigation:
https://bugs.openjdk.java.net/browse/JDK-8061244
The latest webrev of proposed patch is still:
http://cr.openjdk.java.net/~plevart/jdk9-dev/ClassLoader.CNFE/webrev.02/
I invite the public to review it.
Thanks.
Peter
On 10/16/2014 10:44 AM, Peter Levart wrote:
Hi,
As for usage of SharedSecrets, they are not needed to access CNFE
package-private constructor from java.lang.ClassLoader, since they are in
the same package, and from java.net.URLClassLoader, the call to
super.findClass(name) does the job. So here's an updated webrev that also
includes more descriptive message:
http://cr.openjdk.java.net/~plevart/jdk9-dev/ClassLoader.CNFE/webrev.02/
Regards, Peter
On 10/16/2014 09:35 AM, Peter Levart wrote:
On 10/16/2014 01:49 AM, Stanimir Simeonoff wrote:
First, really nice tests.
As for hiding: missing the real classloader impl. might be quite a
bumper
for some middleware implementations. That would make pretty hard to
trace
dependency issues without explicit logging, the latter is usually
available
but still. Also it'd depend if the classloaders actually use
super.findClass() or not.
IMO, the option should be switchable via some system property.
With a little tweak, the message of the stack-less exception thrown
from findClass() methods can be extended to include the classloader's
runtime class name and this message can be inherited by a replacement
stack-full exception. So the stack-trace would look like:
Exception in thread "main" java.lang.ClassNotFoundException: XXX
(thrown by: sun.misc.Launcher$AppClassLoader)
at java.lang.ClassLoader.loadClass(ClassLoader.java:366)
at java.lang.Class.forName0(Native Method)
at java.lang.Class.forName(Class.java:265)
at Test.doIt(Test.java:7)
at Test.main(Test.java:11)
Instead of what we have now:
Exception in thread "main" java.lang.ClassNotFoundException: XXX
at java.net.URLClassLoader.findClass(URLClassLoader.java:381)
at java.lang.ClassLoader.loadClass(ClassLoader.java:426)
at sun.misc.Launcher$AppClassLoader.loadClass(Launcher.java:317)
at java.lang.ClassLoader.loadClass(ClassLoader.java:359)
at java.lang.Class.forName0(Native Method)
at java.lang.Class.forName(Class.java:265)
at Test.doIt(Test.java:7)
at Test.main(Test.java:11)
Would this be enough to cover your concern?
Regards, Peter
I am not sure about the need to hide the stackless c-tor as the effect
can
be achieved by overriding fillInStackTrace(); w/o the extra baggage of
JavaLangAccess.
Overall very decent improvement.
Cheers
Stanimir
