Hi Konstantin,

thanks for the feedback. More inline.

Am 25.01.2016 um 03:51 schrieb Konstantin Kolinko:
2016-01-23 15:17 GMT+03:00 Rainer Jung <rainer.j...@kippdata.de>:
I observed a bottleneck in WebappClassLoaderBase.filter() during a stress
test. The reason is, that the method is synchronized. It looks to me, that
the reason for the synchronization is only the access to the non-thread safe
Matchers packageTriggersPermit and packageTriggersDeny. Since they are not
used anywhere else, IMHO one can get away with using a separate Lock object
for access to the two. E.g.


1. Instead of a lock it is possible to create a new Matcher object on
each invocation.

For performance reasons - this is a hot code path - I would actually prefer the variant I sent later. The hand written code that doesn't use regexp (in a variant that uses enums instead of the "*_or_*" boolean variables. Probably also adjusted due to your remark about the use of "." and "/".

2. There is no need for those Matcher / new Lock object to be
"protected". An interested party can overwrite the whole filter()
method.

Right. With the hand written code they would be completely gone.

3. Filtering resources and classes with the same regexp looks wrong.

It is safe to use that regexp on class names (those do not have '/'
character in them), but it is wrong to use it on resources ('.' is a
valid character in a file name, not a package separator).

The hand written code no longer supports filtering names with mixed "." and "/". I guess those were never intended to get filtered.

If we want to make it even more correct, namely using "." when filtering class names and "/" when filtering resources (I hope that would be correct), the filter() method would need a flag to indicate which case we are in. That would mean changing the signature of a protected method. I guess we need to keep the old signature additionally for compatibility.

4. Generally (if I remember correctly) this filter
a) saves people that do a stupid thing of trying to bundle API classes
or Tomcat classes in their war (e.g. due to misconfiguration of Maven
dependencies)

b) allows a performance improvement, as the search for those classes
in web application classloader can be skipped

It doesn't skip, but it delegates to the parent first for the filtered names, where it most likely will find the searched items.

I forgot whether there were also spec reasons. You indicate, that the reasons are not spec compliance but performance.

Regards,

Rainer

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org

Reply via email to