Mailing-List: contact dev-help@tomcat.apache.org; run by ezmlm
Precedence: bulk
Reply-To: "Tomcat Developers List" <dev@tomcat.apache.org>
Received-SPF: pass (nike.apache.org: domain of rainer.jung@kippdata.de
 designates 195.227.30.149 as permitted sender)
Message-ID: <4F142DAB.8030808@kippdata.de>
Date: Mon, 16 Jan 2012 15:01:15 +0100
From: Rainer Jung <rainer.jung@kippdata.de>
User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64;
 rv:9.0) Gecko/20111222 Thunderbird/9.0.1
MIME-Version: 1.0
To: dev@tomcat.apache.org
Subject: Re: More Caching for WebappClassLoader?
References: <4F0C166B.3010806@kippdata.de>
 <CABzHfV=5hg6mis6LLYDcKtoX+oN0HKx5-aBWuCds2QTDonYOEQ@mail.gmail.com>
In-Reply-To: 
 <CABzHfV=5hg6mis6LLYDcKtoX+oN0HKx5-aBWuCds2QTDonYOEQ@mail.gmail.com>
Content-Type: text/plain; charset=ISO-8859-15; format=flowed
Content-Transfer-Encoding: 7bit

On 11.01.2012 03:16, Konstantin Kolinko wrote:
> 2012/1/10 Rainer Jung<rainer.jung@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-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org