Bug 52999 - Performance issue with locking in ContainerBase.fireContainerEvent()
Summary: Performance issue with locking in ContainerBase.fireContainerEvent()
Status: RESOLVED FIXED
Alias: None
Product: Tomcat 7
Classification: Unclassified
Component: Catalina (show other bugs)
Version: 7.0.26
Hardware: PC Windows XP
: P2 enhancement (vote)
Target Milestone: ---
Assignee: Tomcat Developers Mailing List
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-03-28 21:31 UTC by Konstantin Kolinko
Modified: 2012-06-12 12:40 UTC (History)
0 users



Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Konstantin Kolinko 2012-03-28 21:31:42 UTC
This was reported on dev list [1]:

> 2) org.apache.catalina.core.ContainerBase.fireContainerEvent;
> That method contains critical section:
>  synchronized (listeners) {
>        list = listeners.toArray(list);
> }
>
> Is is called pretty often with every put operation into request or
> session map. That code in tomcat looks like a candidate for
> CopyOnWriteArrayList
>

I confirm that I see fireContainerEvent() calls in many places in StandardSession.

Moreover those are two nested loops:  a loop in StandardSession over array
of context.getApplicationEventListeners(); x copying the list of container listeners inside into array in context.fireContainerEvent().

I cannot confirm reported problem with request attributes - I do not see anything in the code that would send events from that access.

Is it possible to solve it with a ReadWriteLock?
Or it would be better to have a helper class that avoids copying the array over on every access (the said copy-on-write one)?

I classify this as an enhancement request.

[1] Thread "Two performance problems (found during myfaces testing)" on dev list, starting on 2012-03-08,
- http://tomcat.markmail.org/thread/7bbvzmkvyvryvn44
- http://marc.info/?t=133124021200002&r=1&w=2
Comment 1 Mark Thomas 2012-06-12 12:40:15 UTC
The OP may have meant context attributes rather than request attributes.

I'm tempted to remove those container events for performance reasons.

I'm in favour of the ReadWriteLock since there is less copying and therefore fewer objects and hence less GC.

Fixed in trunk and 7.0.x and will be included in 7.0.28 onwards.