Mailing-List: contact dev-help@harmony.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@harmony.apache.org
Received-SPF: pass (herse.apache.org: domain of gcjhd-harmony-dev@m.gmane.org
 designates 80.91.229.2 as permitted sender)
To: dev@harmony.apache.org
From: Egor Pasko <egor.pasko@gmail.com>
Subject: Re: Remove VM statistics or make it thread safe?
Date: 11 Jan 2007 17:58:32 +0600
Lines: 103
Message-ID: <uejq1bwrb.fsf@gmail.com>
References: <300B70C65DCD59468957941A60716EBC486DBD@mssmsx411>
 <ur6u1bz5u.fsf@gmail.com>
 <5836de490701110313y6bb4bfe5ma0d55a23c320696@mail.gmail.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
User-Agent: Gnus/5.09 (Gnus v5.9.0) Emacs/21.3
Sender: news <news@sea.gmane.org>

On the 0x25B day of Apache Harmony Aleksey Ignatenko wrote:
> What I see in the code beyond is that there is no crash possibility and it
> could lead only to fault stat values. But it could be important if it adds
> additional noise of measuring to Thread Checker tool then it should be fixed
> to move next and find really importent hidden race conditions.

is there any possibility to filter out something in Thread Checker?

> Aleksey.
> 
> 
> On 11 Jan 2007 17:06:37 +0600, Egor Pasko <egor.pasko@gmail.com> wrote:
> >
> > 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
> >
> >

-- 
Egor Pasko