From: Stefan Ring Date: Thu, 1 Dec 2011 11:45:42 +0000 (+0100) Subject: PR163: descriptor_params_from_paramtypes is protected by a mutex now (per descriptor... X-Git-Url: http://wien.tomnetworks.com/gitweb/?p=cacao.git;a=commitdiff_plain;h=4e25f6be9878154a9a7ae28917ed34427cb8ca6a PR163: descriptor_params_from_paramtypes is protected by a mutex now (per descriptor pool). 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 de3e44d40..2748ac38d 100644 --- 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) { + size += sizeof(Mutex); /* prepend Mutex */ pool->descriptors = MNEW(u1, size); + /* call Mutex constructor */ + new (reinterpret_cast(pool->descriptors)) Mutex; + pool->descriptors += sizeof(Mutex); 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; + md->pool_lock = reinterpret_cast(pool->descriptors - sizeof(Mutex)); 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 + + /* params already initialized; no need to lock */ + md->pool_lock = NULL; } 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 ******************************************* - 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 - 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. @@ -1045,13 +1052,20 @@ descriptor_pool_parse_method_descriptor(descriptor_pool *pool, utf *desc, void descriptor_params_from_paramtypes(methoddesc *md, s4 mflags) { - typedesc *td; + bool has_lock = md->pool_lock; 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); - td = md->paramtypes; + typedesc *td = md->paramtypes; /* check for `this' pointer */ @@ -1108,6 +1122,9 @@ void descriptor_params_from_paramtypes(methoddesc *md, s4 mflags) 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 935f81699..e1d0bc2a2 100644 --- 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] */ + Mutex *pool_lock; /* synchronizes access to params */ 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 0ee6d19b5..0d95b2070 100644 --- 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. */ - 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. */ diff --git a/src/vm/jit/parse.cpp b/src/vm/jit/parse.cpp index 84b490cd0..caee09557 100644 --- a/src/vm/jit/parse.cpp +++ b/src/vm/jit/parse.cpp @@ -1220,8 +1220,7 @@ jsr_tail: 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; @@ -1253,8 +1252,7 @@ invoke_nonstatic_method: 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); diff --git a/src/vm/jit/verify/typecheck-common.cpp b/src/vm/jit/verify/typecheck-common.cpp index 84505db53..cc39a1073 100644 --- 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 */ - 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 */ diff --git a/src/vm/jit/verify/typecheck-invoke.inc b/src/vm/jit/verify/typecheck-invoke.inc index 47fffd0c0..781defe65 100644 --- a/src/vm/jit/verify/typecheck-invoke.inc +++ b/src/vm/jit/verify/typecheck-invoke.inc @@ -100,10 +100,9 @@ /* 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 */ diff --git a/src/vm/jit/verify/typecheck-stackbased.cpp b/src/vm/jit/verify/typecheck-stackbased.cpp index 67f27350b..a6e658e24 100644 --- 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 */ - 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 */ diff --git a/src/vm/loader.cpp b/src/vm/loader.cpp index 0dc53d0c0..391fb71b5 100644 --- 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->pool_lock = NULL; /* create methodinfo */ diff --git a/src/vm/method.cpp b/src/vm/method.cpp index 7adb88a84..337eb27bc 100644 --- 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? */ - if (md->params == NULL) - descriptor_params_from_paramtypes(md, m->flags); + descriptor_params_from_paramtypes(md, m->flags); paramcount = md->paramcount; @@ -771,8 +770,7 @@ java_handle_objectarray_t *method_get_parametertypearray(methodinfo *m) /* 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; diff --git a/src/vm/resolve.cpp b/src/vm/resolve.cpp index a5345af71..9d6644231 100644 --- 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. */ - 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 */ @@ -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. */ - if (!mi->parseddesc->params) - descriptor_params_from_paramtypes(mi->parseddesc, mi->flags); + descriptor_params_from_paramtypes(mi->parseddesc, mi->flags); /* cache the resolution */ @@ -2675,10 +2673,9 @@ unresolved_method * resolve_create_unresolved_method(classinfo *referer, #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);