Bug 68068 - Hotspot in Ast*Nodes: itable method calls
Summary: Hotspot in Ast*Nodes: itable method calls
Status: RESOLVED FIXED
Alias: None
Product: Tomcat 9
Classification: Unclassified
Component: EL (show other bugs)
Version: 9.0.x
Hardware: PC All
: P2 normal (vote)
Target Milestone: -----
Assignee: Tomcat Developers Mailing List
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2023-11-03 17:39 UTC by John Engebretson
Modified: 2024-02-07 16:56 UTC (History)
1 user (show)



Attachments
Proposed patch - Tomcat 11 - v1 (1.46 KB, patch)
2023-11-07 10:47 UTC, Mark Thomas
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description John Engebretson 2023-11-03 17:39:33 UTC
In-depth profiling of a high-volume, performance-sensitive application identified an avoidable use of an itable method call, a.k.a. invoking an interface method rather than a static or virtual method.  The performance impact is well described at https://stackoverflow.com/questions/21679998/does-it-matter-for-runtime-performance-if-a-method-is-called-by-its-explicit-typ#:~:text=invokeinterface%20is%20known%20to%20be,to%20prefer%20invokevirtual%20to%20invokeinterface%20

The underlying problem is calls to Node.getValue(), such as in AstAnd.getValue():

Object obj = children[0].getValue(ctx);

where "children" is "Node[]".  There are roughly 30 distinct implementations of Node and no JIT is capable of optimizing those away.

Fortunately, all 30 of the Node implementation extends the class SimpleNode.  Changing the references to Node to use SimpleNode will enable JVMs to instead use invokevirtual.

Our tools struggle to measure the actual impact of this issue, because it occurs in small amounts in so many different code paths, and occurs repeatedly in each layer of a nested AST evaluation.  My best guess is 0.2% cpu, directly on the latency critical path.

I hope the fix is as simple as it seems.  :)
Comment 1 John Engebretson 2023-11-06 13:34:11 UTC
Perhaps a simpler solution is to change Node from an interface to an abstract class, then have SimpleNode extends Node rather than implement it.  This achieves the same ends with more targeted and isolated changes.
Comment 2 Mark Thomas 2023-11-07 10:47:20 UTC
Created attachment 39353 [details]
Proposed patch - Tomcat 11 - v1

This is complicated by Node being generated code.

Since there isn't a test to validate the fix, is the attached what is being requested?
Comment 3 John Engebretson 2023-11-07 13:41:55 UTC
Yes, that's perfect, thanks!
Comment 4 Mark Thomas 2023-11-07 16:56:33 UTC
Fixed in:
- 11.0.x for 11.0.0-M14 onwards
- 10.1.x for 10.1.16 onwards
-  9.0.x for  9.0.83 onwards
-  8.5.x for  8.5.96 onwards
Comment 5 John Engebretson 2024-02-07 16:56:00 UTC
Production results confirm a small improvement - greater than zero but not enormous.  Sorry, I'm not able to provide hard numbers because of the huge number of distinct code paths.