PR163: descriptor_params_from_paramtypes is protected by a mutex now (per descriptor...

By careful use of StoreStore and ReadRead memory barriers, we could also use a
kind of double-checked locking on the params field, but the potential
performance gain is definitely not worth the trouble (and risk).

diff --git a/src/vm/descriptor.cpp b/src/vm/descriptor.cpp
index de3e44d40f1194433a8a10e1f77327640d38f249..2748ac38d85931064e1c0ed4884dd2e05a1ed2af 100644 (file)
--- a/src/vm/descriptor.cpp
+++ b/src/vm/descriptor.cpp
@@ -771,7 +771,11 @@ descriptor_pool_alloc_parsed_descriptors(descriptor_pool *pool)
 
        pool->descriptorsize = size;
        if (size) {
 
        pool->descriptorsize = size;
        if (size) {

+               size += sizeof(Mutex);  /* prepend Mutex */

                pool->descriptors = MNEW(u1, size);
                pool->descriptors = MNEW(u1, size);

+               /* call Mutex constructor */
+               new (reinterpret_cast<Mutex*>(pool->descriptors)) Mutex;
+               pool->descriptors += sizeof(Mutex);

                pool->descriptors_next = pool->descriptors;
        }
 
                pool->descriptors_next = pool->descriptors;
        }
 


@@ -921,6 +925,7 @@ descriptor_pool_parse_method_descriptor(descriptor_pool *pool, utf *desc,
        assert(d);
 
        md = (methoddesc *) pool->descriptors_next;
        assert(d);
 
        md = (methoddesc *) pool->descriptors_next;

+       md->pool_lock = reinterpret_cast<Mutex*>(pool->descriptors - sizeof(Mutex));

        pool->descriptors_next += sizeof(methoddesc) - sizeof(typedesc);
 
        utf_ptr = desc->text + 1; /* skip '(' */
        pool->descriptors_next += sizeof(methoddesc) - sizeof(typedesc);
 
        utf_ptr = desc->text + 1; /* skip '(' */


@@ -1010,6 +1015,9 @@ descriptor_pool_parse_method_descriptor(descriptor_pool *pool, utf *desc,
                                        md_param_alloc(md);
                        }
 #endif
                                        md_param_alloc(md);
                        }
 #endif

+
+               /* params already initialized; no need to lock */
+               md->pool_lock = NULL;

        }
        else {
                /* params will be allocated later by
        }
        else {
                /* params will be allocated later by


@@ -1027,13 +1035,12 @@ descriptor_pool_parse_method_descriptor(descriptor_pool *pool, utf *desc,
 
 /* descriptor_params_from_paramtypes *******************************************
  
 
 /* descriptor_params_from_paramtypes *******************************************
  

-   Create the paramdescs for a method descriptor. This function is called
-   when we know whether the method is static or not. This function may only
-   be called once for each methoddesc, and only if md->params == NULL.
+   Create the paramdescs for a method descriptor. This function is
+   called when we know whether the method is static or not. This
+   function does nothing if md->params != NULL (checked atomically).

 
    IN:
        md...............the parsed method descriptor
 
    IN:
        md...............the parsed method descriptor

-                           md->params MUST be NULL.

           mflags...........the ACC_* access flags of the method. Only the
                            ACC_STATIC bit is checked.
                                                The value ACC_UNDEF is NOT allowed.
           mflags...........the ACC_* access flags of the method. Only the
                            ACC_STATIC bit is checked.
                                                The value ACC_UNDEF is NOT allowed.


@@ -1045,13 +1052,20 @@ descriptor_pool_parse_method_descriptor(descriptor_pool *pool, utf *desc,
 
 void descriptor_params_from_paramtypes(methoddesc *md, s4 mflags)
 {
 
 void descriptor_params_from_paramtypes(methoddesc *md, s4 mflags)
 {

-       typedesc *td;
+       bool has_lock = md->pool_lock;

 
        assert(md);
 
        assert(md);

+       if (md->pool_lock)
+               md->pool_lock->lock();
+       if (md->params) {
+               if (has_lock)
+                       md->pool_lock->unlock();
+               return;
+       }

        assert(md->params == NULL);
        assert(mflags != ACC_UNDEF);
 
        assert(md->params == NULL);
        assert(mflags != ACC_UNDEF);
 

-       td = md->paramtypes;
+       typedesc *td = md->paramtypes;

 
        /* check for `this' pointer */
 
 
        /* check for `this' pointer */
 


@@ -1108,6 +1122,9 @@ void descriptor_params_from_paramtypes(methoddesc *md, s4 mflags)
                                md_param_alloc(md);
                }
 #endif
                                md_param_alloc(md);
                }
 #endif

+
+       if (has_lock)
+               md->pool_lock->unlock();

 }
 
 
 }
 
 

diff --git a/src/vm/descriptor.hpp b/src/vm/descriptor.hpp
index 935f81699727307571dcb2e33d9d57553b556189..e1d0bc2a21eb6ade9b3125b4e3e5709c00ce3c1c 100644 (file)
--- a/src/vm/descriptor.hpp
+++ b/src/vm/descriptor.hpp
@@ -128,6 +128,7 @@ struct methoddesc {
 #endif
        s4         memuse;          /* number of stack slots used                 */
        paramdesc *params;          /* allocated parameter descriptions [3]       */
 #endif
        s4         memuse;          /* number of stack slots used                 */
        paramdesc *params;          /* allocated parameter descriptions [3]       */

+       Mutex     *pool_lock;       /* synchronizes access to params              */

        typedesc   returntype;      /* parsed descriptor of the return type       */
        typedesc   paramtypes[1];   /* parameter types, variable length!          */
 };
        typedesc   returntype;      /* parsed descriptor of the return type       */
        typedesc   paramtypes[1];   /* parameter types, variable length!          */
 };

diff --git a/src/vm/jit/optimizing/bytecode_escape.c b/src/vm/jit/optimizing/bytecode_escape.c
index 0ee6d19b5b8a6eff77961ec879e48b94aeb65c33..0d95b2070f27b0503391cae15c7e9e314b92012b 100644 (file)
--- a/src/vm/jit/optimizing/bytecode_escape.c
+++ b/src/vm/jit/optimizing/bytecode_escape.c
@@ -775,8 +775,7 @@ static void bc_escape_analysis_parse_invoke(bc_escape_analysis_t *be, jcode_t *j
 
        /* Parse parameters if not done yet. */
 
 
        /* Parse parameters if not done yet. */
 

-       if (md->params == NULL)
-               descriptor_params_from_paramtypes(md, opc == BC_invokestatic ? ACC_STATIC : 0);
+       descriptor_params_from_paramtypes(md, opc == BC_invokestatic ? ACC_STATIC : 0);

 
        /* Try to lazyly resolve method. */
 
 
        /* Try to lazyly resolve method. */
 

diff --git a/src/vm/jit/parse.cpp b/src/vm/jit/parse.cpp
index 84b490cd0297e4430a32fe3f1c907ee818c0f1b9..caee0955745fdab5bb1f53578e5822865eb86813 100644 (file)
--- a/src/vm/jit/parse.cpp
+++ b/src/vm/jit/parse.cpp
@@ -1220,8 +1220,7 @@ jsr_tail:
 
                        md = fmi->parseddesc.md;
 
 
                        md = fmi->parseddesc.md;
 

-                       if (md->params == NULL)
-                               descriptor_params_from_paramtypes(md, ACC_STATIC);
+                       descriptor_params_from_paramtypes(md, ACC_STATIC);

 
                        goto invoke_method;
 
 
                        goto invoke_method;
 


@@ -1253,8 +1252,7 @@ invoke_nonstatic_method:
 
                        md = fmi->parseddesc.md;
 
 
                        md = fmi->parseddesc.md;
 

-                       if (md->params == NULL)
-                               descriptor_params_from_paramtypes(md, 0);
+                       descriptor_params_from_paramtypes(md, 0);

 
 invoke_method:
                        code_unflag_leafmethod(code);
 
 invoke_method:
                        code_unflag_leafmethod(code);

diff --git a/src/vm/jit/verify/typecheck-common.cpp b/src/vm/jit/verify/typecheck-common.cpp
index 84505db5304e6e00520f85609d11b7b4fd7ae8df..cc39a1073e60a2690fea18e2c44b622b28f0fc6d 100644 (file)
--- a/src/vm/jit/verify/typecheck-common.cpp
+++ b/src/vm/jit/verify/typecheck-common.cpp
@@ -493,8 +493,7 @@ bool typecheck_init_locals(verifier_state *state, bool newthis)
 
        /* allocate parameter descriptors if necessary */
 
 
        /* allocate parameter descriptors if necessary */
 

-       if (!state->m->parseddesc->params)
-               descriptor_params_from_paramtypes(state->m->parseddesc,state->m->flags);
+       descriptor_params_from_paramtypes(state->m->parseddesc, state->m->flags);

 
        /* pre-initialize variables as TYPE_VOID */
 
 
        /* pre-initialize variables as TYPE_VOID */
 

diff --git a/src/vm/jit/verify/typecheck-invoke.inc b/src/vm/jit/verify/typecheck-invoke.inc
index 47fffd0c08dc8945bab770aee9e0ebbc5daaac7d..781defe657b27190610ca85cf0a7b801ac1168d0 100644 (file)
--- a/src/vm/jit/verify/typecheck-invoke.inc
+++ b/src/vm/jit/verify/typecheck-invoke.inc
@@ -100,10 +100,9 @@
 
        /* allocate parameters if necessary */
        
 
        /* allocate parameters if necessary */
        

-       if (!md->params)
-               descriptor_params_from_paramtypes(
-                       md,
-                       (invokestatic) ? ACC_STATIC : ACC_NONE);
+       descriptor_params_from_paramtypes(
+               md,
+               (invokestatic) ? ACC_STATIC : ACC_NONE);

 
        /* check parameter types */
 
 
        /* check parameter types */
 

diff --git a/src/vm/jit/verify/typecheck-stackbased.cpp b/src/vm/jit/verify/typecheck-stackbased.cpp
index 67f27350bb8067b2d76a4f9884567f1c6b67fd12..a6e658e2475479b646f78734bd0c970e883b7e49 100644 (file)
--- a/src/vm/jit/verify/typecheck-stackbased.cpp
+++ b/src/vm/jit/verify/typecheck-stackbased.cpp
@@ -732,8 +732,7 @@ bool typecheck_stackbased(jitdata *jd)
 
        /* allocate parameter descriptors if necessary */
 
 
        /* allocate parameter descriptors if necessary */
 

-       if (!state.m->parseddesc->params)
-               descriptor_params_from_paramtypes(state.m->parseddesc,state.m->flags);
+       descriptor_params_from_paramtypes(state.m->parseddesc, state.m->flags);

 
        /* allocate the stack buffers */
 
 
        /* allocate the stack buffers */
 

diff --git a/src/vm/loader.cpp b/src/vm/loader.cpp
index 0dc53d0c0330602339dbad0aae5934ce9589e5a0..391fb71b5c7ace540084bbd44e66185e4939d760 100644 (file)
--- a/src/vm/loader.cpp
+++ b/src/vm/loader.cpp
@@ -2119,6 +2119,7 @@ classinfo *load_newly_created_array(classinfo *c, classloader_t *loader)
        clonedesc->paramslots = 0;
        clonedesc->paramtypes[0].classref = classrefs + 0;
        clonedesc->params = NULL;
        clonedesc->paramslots = 0;
        clonedesc->paramtypes[0].classref = classrefs + 0;
        clonedesc->params = NULL;

+       clonedesc->pool_lock = NULL;

 
        /* create methodinfo */
 
 
        /* create methodinfo */
 

diff --git a/src/vm/method.cpp b/src/vm/method.cpp
index 7adb88a84afd9ae03b874905e23e7f930a7868f4..337eb27bcd4e7e14339e206977677d4ca8637127 100644 (file)
--- a/src/vm/method.cpp
+++ b/src/vm/method.cpp
@@ -735,8 +735,7 @@ int32_t method_get_parametercount(methodinfo *m)
        
        /* is the descriptor fully parsed? */
 
        
        /* is the descriptor fully parsed? */
 

-       if (md->params == NULL)
-               descriptor_params_from_paramtypes(md, m->flags);
+       descriptor_params_from_paramtypes(md, m->flags);

 
        paramcount = md->paramcount;
 
 
        paramcount = md->paramcount;
 


@@ -771,8 +770,7 @@ java_handle_objectarray_t *method_get_parametertypearray(methodinfo *m)
 
        /* is the descriptor fully parsed? */
 
 
        /* is the descriptor fully parsed? */
 

-       if (m->parseddesc->params == NULL)
-               descriptor_params_from_paramtypes(md, m->flags);
+       descriptor_params_from_paramtypes(md, m->flags);

 
        paramtypes = md->paramtypes;
        paramcount = md->paramcount;
 
        paramtypes = md->paramtypes;
        paramcount = md->paramcount;

diff --git a/src/vm/resolve.cpp b/src/vm/resolve.cpp
index a5345af71f926cc5c23baf8988fb12020a8e1166..9d66442315e2a5a88e7bb2a355e429e6bd638a4c 100644 (file)
--- a/src/vm/resolve.cpp
+++ b/src/vm/resolve.cpp
@@ -2067,8 +2067,7 @@ resolve_result_t resolve_method_lazy(methodinfo *refmethod,
 
        /* have the method params already been parsed? no, do it. */
 
 
        /* have the method params already been parsed? no, do it. */
 

-       if (!mi->parseddesc->params)
-               descriptor_params_from_paramtypes(mi->parseddesc, mi->flags);
+       descriptor_params_from_paramtypes(mi->parseddesc, mi->flags);

 
        /* cache the result of the resolution */
 
 
        /* cache the result of the resolution */
 


@@ -2190,8 +2189,7 @@ bool resolve_method(unresolved_method *ref, resolve_mode_t mode, methodinfo **re
 
        /* have the method params already been parsed? no, do it. */
 
 
        /* have the method params already been parsed? no, do it. */
 

-       if (!mi->parseddesc->params)
-               descriptor_params_from_paramtypes(mi->parseddesc, mi->flags);
+       descriptor_params_from_paramtypes(mi->parseddesc, mi->flags);

 
        /* cache the resolution */
 
 
        /* cache the resolution */
 


@@ -2675,10 +2673,9 @@ unresolved_method * resolve_create_unresolved_method(classinfo *referer,
 #endif
 
        /* allocate params if necessary */
 #endif
 
        /* allocate params if necessary */

-       if (!methodref->parseddesc.md->params)
-               descriptor_params_from_paramtypes(
-                       methodref->parseddesc.md,
-                       (invokestatic) ? ACC_STATIC : ACC_NONE);
+       descriptor_params_from_paramtypes(
+               methodref->parseddesc.md,
+               (invokestatic) ? ACC_STATIC : ACC_NONE);

 
        /* create the data structure */
        ref = NEW(unresolved_method);
 
        /* create the data structure */
        ref = NEW(unresolved_method);