harmony-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Egor Pasko <egor.pa...@gmail.com>
Subject Re: Remove VM statistics or make it thread safe?
Date Thu, 11 Jan 2007 11:06:37 GMT
On the 0x25B day of Apache Harmony Ilia A. Leviev 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.

JIT profilers and Jitrino.OPT instruction profiler are thread-unsafe
too. But this is a *feature*. We do not need precise data here at a
cost of extra synchronisations.

Same arguments may be applied to vm stats...

> \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
> 

-- 
Egor Pasko


Mime
View raw message