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 2022/10/04 10:20:37 UTC

[Bug 66294] New: Util.getContextClassLoader() can be a hotspot

https://bz.apache.org/bugzilla/show_bug.cgi?id=66294

            Bug ID: 66294
           Summary: Util.getContextClassLoader() can be a hotspot
           Product: Tomcat 9
           Version: 9.0.65
          Hardware: All
                OS: Linux
            Status: NEW
          Severity: regression
          Priority: P2
         Component: EL
          Assignee: dev@tomcat.apache.org
          Reporter: mxhensel@amazon.de
  Target Milestone: -----

Created attachment 38401
  --> https://bz.apache.org/bugzilla/attachment.cgi?id=38401&action=edit
Benchmark which shows performance difference between getting class loader
with/without privileges

In our application Util.getContextClassLoader() is a hotspot with almost all
the cost coming from AccessController.doPrivileged. By adding instrumentation
we've confirmed that the native implementation of AccessController.doPrivileged
itself and not PrivilegedGetTccl.run() is expensive. In an example request to
our application we had 17000 calls to Util.getContextClassLoader() and recorded
this CPU time in the methods:
0.682 ms for javax.el.Util$PrivilegedGetTccl.run()
27.800 ms for javax.el.Util.getContextClassLoader()

This potential hotspot was introduced four years ago when a change was made to
get the context class loader of the Thread with privileges enabled:
https://bz.apache.org/bugzilla/show_bug.cgi?id=62080

We'd like to fix this hotspot by calling
Thread.currentThread().getContextClassLoader() without privileges enabled
again. This would be a revert of the change here:
https://github.com/apache/tomcat/commit/7c359957f0b600b92e964b1a2497374e6cac4bc3#diff-f5b7c13f66b3d070a6b1c1713ad4b60fd70b26f579a746edc8fe9f19109243b2L93
Since this change addressed a rare bug, we probably have to make this change
backwards compatible by adding a configuration switch.

I have also included a basic benchmark which compares the performance of the
two approaches. The benchmark shows a difference by factor 100. I ran the
benchmark with JDK 8 and 11.

Since SecurityManager and related functionality are on track to deprecation,
this change may be mandatory in the future, irrespective of the performance
benefit:
https://openjdk.org/jeps/411

-- 
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 66294] Util.getContextClassLoader() can be a hotspot

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

--- Comment #1 from Mark Thomas <ma...@apache.org> ---
Given that the SecurityManager is heading towards deprecation, why not just run
without a SecurityManager to avoid this hotspot?

-- 
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 66294] Util.getContextClassLoader() can be a hotspot

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

--- Comment #4 from Maximilian Hensel <mx...@amazon.de> ---
Thanks, that's great to hear.

-- 
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 66294] Util.getContextClassLoader() can be a hotspot

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

Mark Thomas <ma...@apache.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|---                         |FIXED

--- Comment #3 from Mark Thomas <ma...@apache.org> ---
Fixed in:
- 10.1.x for 10.1.2 onwards
-  9.0.x for  9.0.69 onwards
-  8.5.x for  8.5.84 onwards

The privileged block is now disabled by default so this should "just work" once
you upgrade.

-- 
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 66294] Util.getContextClassLoader() can be a hotspot

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

--- Comment #2 from Maximilian Hensel <mx...@amazon.de> ---
(In reply to Mark Thomas from comment #1)
> Given that the SecurityManager is heading towards deprecation, why not just
> run without a SecurityManager to avoid this hotspot?

We've checked this, but unfortunately, it is not possible for us to run without
SecurityManager for non-Tomcat reasons.

-- 
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 66294] Util.getContextClassLoader() can be a hotspot

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

John Engebretson <je...@amazon.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jengebr@amazon.com

-- 
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