harmony-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Mikhail Fursov" <mike.fur...@gmail.com>
Subject Re: Remove VM statistics or make it thread safe?
Date Thu, 11 Jan 2007 11:06:17 GMT
Edge/entry-backedge/value profiles that are used in our default or server
modes are also not thread safe and IMO this is OK.
Not everything must be thread safe. Because the exact numbers are not always
the goal.


On 1/11/07, Leviev, Ilia A <ilia.a.leviev@intel.com> wrote:
>
>
> Hi,
> Recently I have started to use Thread Checker tool for finding unsafe
> thread access places in VM.
> First I have tried to check VM that run simple application such HWA.
> The tool shows that there is thread unsafe access to some of
> VM_Statistics fields that result in race condition.
>
> \vm\vmcore\include\ vm_stats.h
>
> class VM_Statistics
> {
> public:
>
>    uint64 num_free_local_called;
>    uint64 num_jni_handles_wasted_refs;
>    uint64 num_free_local_called_free;
>    uint64 num_local_jni_handles;
>    uint64 num_jni_handles_freed;
>
>       ~~~~~~~~~~~~~~~~~~~~~
>
> The problem occurred during execution of free_local_object_handles2
> function
> from \vm\vmcore\src\object\object_handles.cpp.
> The function increment fields of type uint64.
>
> ////////////////////////////////////////////////////////
> void free_local_object_handles2(ObjectHandles* head)
> {
>     ObjectHandlesNew* h = (ObjectHandlesNew*)head;
>     assert(h);
> #ifdef VM_STATS
>     VM_Statistics::get_vm_stats().num_free_local_called++;// race
> condition
>     if(h->next != NULL)
>         VM_Statistics::get_vm_stats().num_free_local_called_free++;//
> race condition
> #endif //VM_STATS
>     while(h->next) {
> #ifdef VM_STATS
>         unsigned size = h->size;
>         VM_Statistics::get_vm_stats().num_local_jni_handles += size; //
> race condition
>         VM_Statistics::get_vm_stats().num_jni_handles_freed++; // race
> condition
>         VM_Statistics::get_vm_stats().num_jni_handles_wasted_refs +=
> (h->capacity - size);// race condition
> #endif //VM_STATS
>         ObjectHandlesNew* next = h->next;
>         STD_FREE(h);
>         h = next;
>     }
> #ifdef VM_STATS
>     VM_Statistics::get_vm_stats().num_jni_handles_wasted_refs +=
> (h->capacity - h->size); // race condition
> #endif //VM_STATS
> }
> ////////////////////////////////////////////////////////
>
> There are several solutions:
> 1)   remove and do not use statistics at all, if anybody not needs it.
> 2)   make this function synchronized using lock-unlock functions from
> lock_manager,
> 3)   make increment by atomic APR function such apr_atomic_inc32(
> volatile apr_uint32_t *  mem ),
>      but to use it we need to change the type of the fields from uint64
> to volatile uint32.
>
> What solution will be more acceptable?
>
>
> Best regards,
> Leviev Ilya
> Intel Java & XML Engineering
>



-- 
Mikhail Fursov

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message