RFR: 8330851: C2: More efficient TypeFunc creation

[Dean Long]

On Wed, 30 Oct 2024 06:43:23 GMT, Amit Kumar <amitkumar at openjdk.org> wrote:

> Lazy computation of TypeFunc.
> 
> Testing: Tier1 on Fastdebug & Release VMs (`s390x architecture`)

It looks OK, but I think we are paying some overhead every time we try to get the TypeFunc, because C++ has to first check if it's the first time the function was called.  Instead, how about getting rid of the lambda and make the initialization explicit?For example, rename LockNode::lock_type() to LockNode::lock_type_init(), and have it save the result in a static const field.  Then have LockNode::lock_type() simply return the field.

This is what I meant:

diff --git a/src/hotspot/share/opto/callnode.hpp b/src/hotspot/share/opto/callnode.hpp
index 2d3835b71ad..f72e78745b5 100644
--- a/src/hotspot/share/opto/callnode.hpp
+++ b/src/hotspot/share/opto/callnode.hpp
@@ -1190,9 +1190,11 @@ class AbstractLockNode: public CallNode {
 //    2 -   a FastLockNode
 //
 class LockNode : public AbstractLockNode {
+  static const TypeFunc *_lock_type_tf;
 public:
 
-  static const TypeFunc *lock_type() {
+  static void lock_type_init() {
+    assert(_lock_type_tf == nullptr, "lock_type_init() already called");
     // create input type (domain)
     const Type **fields = TypeTuple::fields(3);
     fields[TypeFunc::Parms+0] = TypeInstPtr::NOTNULL;  // Object to be Locked
@@ -1205,7 +1207,12 @@ class LockNode : public AbstractLockNode {
 
     const TypeTuple *range = TypeTuple::make(TypeFunc::Parms+0,fields);
 
-    return TypeFunc::make(domain,range);
+    _lock_type_tf = TypeFunc::make(domain,range);
+  }
+
+  static const TypeFunc *lock_type() {
+    assert(_lock_type_tf != nullptr, "lock_type_init() not called");
+    return _lock_type_tf;
   }
 
   virtual int Opcode() const;

Nice work so far.  I would suggest making  _Type() accessors inlined, and try to reduce boiler-plate code with macros if possible (field name and accessor function name can both be derived from a common root, which is pretty common practice in HotSpot code).

If you move all these accessor functions into the .hpp or .inline.hpp file, so they can be inlined, then I think the benefit of a macro will be come more apparent, but I won't insist.  Let's see what other reviewers think.

-------------

PR Comment: https://git.openjdk.org/jdk/pull/21782#issuecomment-2450827514
PR Comment: https://git.openjdk.org/jdk/pull/21782#issuecomment-2455940131
PR Comment: https://git.openjdk.org/jdk/pull/21782#issuecomment-2458525693
PR Comment: https://git.openjdk.org/jdk/pull/21782#issuecomment-2461241654