On 11.01.2012 03:16, Konstantin Kolinko wrote:
2012/1/10 Rainer Jung<rainer.j...@kippdata.de>:
Note that when looking for a class most time is wasted when looking up
a "*.class" resource. That code is in findResourceInternal() and I
think that that method should be considered as well, to speed up
lookup of any resources, not only the classes.
I had some old patch draft lying around. I submitted it in
https://issues.apache.org/bugzilla/show_bug.cgi?id=52448
See that issue for details.
I started to look at it, but noticed a problem when trying to apply my
naive implementation to resources as well: as long as I stick to
loadClass, I do now, that we are synchronized. So there's no additional
locking overhead. When switching to caching for resources things get
much more complicated, because this simple assumption is no longer true.
I saw you e.g. added a volatile to one member of ResourceEntry because
the WebappCL uses double checked locking in one place etc.
So I would prefer to fix the loadClass() case first using a simple pattern.
Concerning the cache of classes that you are proposing: consider the
following scenario:
(1) WebappClassLoader is asked to load a class. It does not find it
and then finds it in the parent classloader.
(2) It is asked for this class again.
During (2) should it look into its own resources first? If during the
time between (1) and (2) one puts a class file into WEB-INF/classes
should that class file be used?
I think that during (2) call it must ignore the class in
WEB-INF/classes and still use the class from parent classloader. That
is for consistency with previous result.
That would be the behaviour of the implementation I have in mind.
I think that instead of caching the class instance itself it would be
safe to cache the fact that the class was loaded from the parent
classloader. It separates responsibilities and solves the issue with
dynamic classloading. The difference is small though.
Actually WebappClassLoader#notFoundResources already serves similar purpose.
Correct in theory. When I tried to implement this, I noticed that we do
use very different methods to actually retrieve the classes from the CLs:
- findClass() for super
- loadClass() for system
- Class.forName() for parent
Especially for super it would be not clear to me, what the right method
for loading the class in the cached case would be: loadClass() or
findLoadedClass() or Class.forName()? Simply using findClass() again
doesn't seem to be the right thing to make the caching work. The more I
look into it, the more I come back to my initial and simple idea.
Regarding class instances caching I see two concerns:
a) Garbage-collecting unused classes.
b) Hot-swapping classes during debugging.
Regarding a) that is not a concern if parent classloader already has
cache of those classes. Caching just names is safer, does not prevent
gc of the classes and the names take less memory (and no PermGen
memory).
I'll make some experiment to find out, how much memory overhead it would
actually mean.
Regarding b) I suspect that hot-swapping changes bytecode but does not
change the Class instance. Documentation is at [1], but I do not have
much experience with this feature.
[1]
http://docs.oracle.com/javase/1.4.2/docs/guide/jpda/enhancements.html#hotswap
I read the docs and I agree: it seems they swap the actual bytes inside
the classes. It should be transparent to the class loaders.
Regarding "try loading from system loader" call:
Isn't that call fast? In JRE 6 there are some indexes (lib/classlist,
lib/meta-index) that should help system classloader to reject requests
for classes that it cannot load.
It was not the primary bottleneck, but ever now and then even
system.loadClass() showed up int the thread dumps. Unfortunately I don't
have them at hand any longer, so can't inspect, where exactly the code
was captured.
While we are talking about classloaders - there is one more patch in Bugzilla,
https://issues.apache.org/bugzilla/show_bug.cgi?id=48903#c6
that tries to use some jdk7 feature to improve classloader locking.
I do not plan to look at it in near future, as I do not see much
benefit from jdk7 yet.
Just reminding.
I had a look. Again, I wanted to get something done for the original
issue first, but thanks for the hint!
Regards,
Rainer
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
