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

            Bug ID: 69532
           Summary: Performance optimization in
                    Util.getExpressionFactory()
           Product: Tomcat 9
           Version: 9.0.98
          Hardware: PC
            Status: NEW
          Severity: normal
          Priority: P2
         Component: EL
          Assignee: dev@tomcat.apache.org
          Reporter: jeng...@amazon.com
  Target Milestone: -----

Created attachment 39968
  --> https://bz.apache.org/bugzilla/attachment.cgi?id=39968&action=edit
Speed test

A large, latency-sensitive application spends approximately 0.6% of cpu
executing javax.el.Util.getExpressionFactory.  Profiling indicates 2/3rds of
that time is spent acquiring and releasing the read lock associated with each
cache entry.  Specifically, this block of code:

final Lock readLock = cacheValue.getLock().readLock();
readLock.lock();
try {
        factory = cacheValue.getExpressionFactory();
} finally {
        readLock.unlock();
}

However, this read locking is unnecessary, as the protected value is
initialized to null, then set exactly once to a long-lived object.  Unlocked
reads that return a non-null value must be correct, and reads that return a
null value will be resolved immediately afterwards with the write locking:


if (factory == null) {
        final Lock writeLock = cacheValue.getLock().writeLock();
        writeLock.lock();
        try {
                factory = cacheValue.getExpressionFactory();
                if (factory == null) {
                        factory = ExpressionFactory.newInstance();
                        cacheValue.setExpressionFactory(factory);
                }
        } finally {
                writeLock.unlock();
        }
}


Eliminating the read lock doubles the speed of the attached performance test
while maintaining safety (I think - please doublecheck!).  I observe results
similar to:

Done with old in 284
Done with old in 283
Done with old in 279
Done with old in 279
Done with old in 279
Done with new in 204
Done with new in 159
Done with new in 152
Done with new in 151
Done with new in 153

Notes:
1. The test uses 500 distinct classloaders to avoid any bias towards our case
of 1-2 contexts in the map.  I'm not sure what's typical.
2. Run the test with -Xmx1g -Xms1g -XX:+UseParallelGC
3. The test is single-threaded so the lock is uncontended.  Perf impact may
vary under contention.

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org

Reply via email to