You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by bu...@apache.org on 2012/04/16 20:42:12 UTC

DO NOT REPLY [Bug 53085] New: [perf] [concurrency] DefaultInstanceManager.annotationCache is not optimal for more threads

https://issues.apache.org/bugzilla/show_bug.cgi?id=53085

             Bug #: 53085
           Summary: [perf] [concurrency]
                    DefaultInstanceManager.annotationCache is not optimal
                    for more threads
           Product: Tomcat 7
           Version: 7.0.26
          Platform: All
        OS/Version: All
            Status: NEW
          Severity: enhancement
          Priority: P3
         Component: Catalina
        AssignedTo: dev@tomcat.apache.org
        ReportedBy: Martin.Kocicak.Koci@gmail.com
    Classification: Unclassified


Created attachment 28616
  --> https://issues.apache.org/bugzilla/attachment.cgi?id=28616
Screenshot : Thread states during test

This is a small Concurrency problem also find during myfaces performance
testing [1]

org.apache.catalina.core.DefaultInstanceManager.annotationCache uses
WeakHashMap for cache, but this approach needs sychronized get() access. This
can lead to complete thread stuck at this lock - see attached screenshots from
yourkit profiler. 

I think this problem is classical concurrent cache problem = many read but only
few put()s, reads vastly outnumber writes. It this case is should be read
without locking otherwise it is a concurrency bottleneck.

I didn't check the code of DefaultInstanceManager deeply, following are
suggestions only:
1) Normally is this concurrent-cache solvable with ConcurrentHashMap or with
maps based on this type like [2] or [3]
2) the 'weakness' can achieved with String (classname) if it is acceptable to
maintain annotations for class that can be already unloaded or with
WeakReference

[1] http://tomcat.markmail.org/thread/7bbvzmkvyvryvn44
[2]
http://svn.apache.org/repos/asf/lucene/dev/trunk/solr/core/src/java/org/apache/solr/util/ConcurrentLFUCache.java
[3] http://code.google.com/p/guava-libraries/wiki/CachesExplained

-- 
Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

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


DO NOT REPLY [Bug 53085] [perf] [concurrency] DefaultInstanceManager.annotationCache is not optimal for more threads

Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=53085

--- Comment #2 from Konstantin Kolinko <kn...@gmail.com> 2012-04-18 20:37:31 UTC ---
(In reply to comment #0)
> 2) the 'weakness' can achieved with String (classname) if it is acceptable to
> maintain annotations for class that can be already unloaded or with
> WeakReference

It cannot be String alone. You need {ClassLoader + String} to identify a class.

-- 
Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

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


DO NOT REPLY [Bug 53085] [perf] [concurrency] DefaultInstanceManager.annotationCache is not optimal for more threads

Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=53085

--- Comment #1 from Martin Kočí <Ma...@gmail.com> 2012-04-16 18:43:44 UTC ---
Created attachment 28617
  --> https://issues.apache.org/bugzilla/attachment.cgi?id=28617
Screenshot: Monitor Usage during test

-- 
Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[Bug 53085] [perf] [concurrency] DefaultInstanceManager.annotationCache is not optimal for more threads

Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=53085

--- Comment #6 from Christopher Schultz <ch...@christopherschultz.net> ---
(In reply to Manoj from comment #5)
> Which tomcat build have this patch?

Read the comments: the support class was added to the trunk back in 2012 and
presumably appears in all releases following that date.

That support class, however, is not actually being used in any released
version. If you want to enable it, you'll have to manually apply Konstantin's
patch to the Tomcat sources and build it yourself. (Fortunately, building
Tomcat has become staggeringly easy since Tomcat 7.0.x).

-- 
You are receiving this mail because:
You are the assignee for the bug.

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


[Bug 53085] [perf] [concurrency] DefaultInstanceManager.annotationCache is not optimal for more threads

Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=53085

--- Comment #4 from Manoj <ma...@fisglobal.com> ---
Am also facing similar issue. This issue stalls the application throughput
after a certain load. When can we expect a patch for annotationCache
concurrency?

-- 
You are receiving this mail because:
You are the assignee for the bug.

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


[Bug 53085] [perf] [concurrency] DefaultInstanceManager.annotationCache is not optimal for more threads

Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=53085

--- Comment #7 from Manoj <ma...@fisglobal.com> ---
Looks like the support class is only available in tomcat 8. Can we have the fix
added in tomcat 7?

-- 
You are receiving this mail because:
You are the assignee for the bug.

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


DO NOT REPLY [Bug 53085] [perf] [concurrency] DefaultInstanceManager.annotationCache is not optimal for more threads

Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=53085

--- Comment #3 from Konstantin Kolinko <kn...@gmail.com> 2012-04-19 12:19:47 UTC ---
Created attachment 28641
  --> https://issues.apache.org/bugzilla/attachment.cgi?id=28641
2012-04-19_tc8_53085_DefaultInstanceManager.patch

I added ManagedConcurrentWeakHashMap class to trunk in r1327915

Here is a patch for trunk that uses that new class to fix this issue. I have
not committed it yet.

Things that I am not sure:

1. Should DefaultInstanceManager implement LifecycleListener just to catch
periodic events?

There is PeriodicEventListener interface that seems more suitable.

The only "problem" though is ugliness in TestDefaultInstanceManager part of the
patch where I need to call this maintenance processing. Simulating a
LifecycleEvent is more than it is worth.

The good thing though is that I do not need to override
StandardContext#backgroundProcessing() to fire the event, nor invent a
LifecycleListener -> PeriodicEventListener adapter.

2. Looking for calls of DefaultInstanceManager constructor, there are 3 such
places. Only the instanceManager instance owned by StandardContext is handled
by this patch.  I wonder whether 2 other places are actually needed.

Maybe amend the Context interface by adding #getInstanceManager() method, at
least in Tomcat 8?

The 3 places:
 - StandardContext#startInternal()
 - ApplicationFilterConfig#getInstanceManager()
 - AsyncContextImpl#getInstanceManager()

-- 
Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

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


[Bug 53085] [perf] [concurrency] DefaultInstanceManager.annotationCache is not optimal for more threads

Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=53085

--- Comment #5 from Manoj <ma...@fisglobal.com> ---
Which tomcat build have this patch?

-- 
You are receiving this mail because:
You are the assignee for the bug.

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