harmony-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Regis <xu.re...@gmail.com>
Subject Re: call for review: (HARMONY-6257) [classlib][luni] - Optimize OSMemory.get/setByteArray
Date Tue, 04 Aug 2009 04:45:58 GMT
Pavel Pervov wrote:
> Hi, Regis.
> 
> In Harmony VM Get/ReleasePrimitiveArrayCritical are implemented by
> pinning/unpinning array object (see
> working_vm/vm/vmcore/src/jni/jni.cpp, lines 1377 and 1415). The
> complexity of the operation depends on GC implementation. You can
> estimate Harmony VM times by looking at the implementation of
> gc_pin_object/gc_unpin_object under working_vm/vm/gc_gen/.
> 
> WBR,
>     Pavel.
> 

I searched "gc_pin_object" in "working_vm/vm", seems there is only one 
implementation in "working_vm/vm/gc_gen/src/common/gc_for_vm.cpp":

Boolean gc_is_object_pinned (Managed_Object_Handle obj)
{  return 0; }

void gc_pin_object (Managed_Object_Handle* p_object)
{  return; }

void gc_unpin_object (Managed_Object_Handle* p_object)
{  return; }

seems drlvm is not support pin object currently, so there is no difference 
between Get/ReleasePrimitiveArrayCritical and 
Get/Release<PrimitiveType>ArrayElements.


> On Mon, Aug 3, 2009 at 10:29 AM, Regis<xu.regis@gmail.com> wrote:
>> Regis wrote:
>>> Jimmy,Jing Lv wrote:
>>>> Hi,
>>>>
>>>> 2009/7/9 Regis <xu.regis@gmail.com>
>>>>
>>>>> Regis wrote:
>>>>>
>>>>>> Regis Xu (JIRA) wrote:
>>>>>>
>>>>>>>    [
>>>>>>>
>>>>>>> https://issues.apache.org/jira/browse/HARMONY-6257?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel]
>>>>>>>
>>>>>>> Regis Xu updated HARMONY-6257:
>>>>>>> ------------------------------
>>>>>>>
>>>>>>>   Attachment: HARMONY-6257.diff
>>>>>>>
>>>>>>>  [classlib][luni] - Optimize OSMemory.get/setByteArray
>>>>>>>> -----------------------------------------------------
>>>>>>>>
>>>>>>>>               Key: HARMONY-6257
>>>>>>>>               URL: https://issues.apache.org/jira/browse/HARMONY-6257
>>>>>>>>           Project: Harmony
>>>>>>>>        Issue Type: Improvement
>>>>>>>>        Components: Classlib
>>>>>>>>  Affects Versions: 5.0M10
>>>>>>>>          Reporter: Regis Xu
>>>>>>>>       Attachments: HARMONY-6257.diff
>>>>>>>>
>>>>>>>>
>>>>>>>> in getByteArray, use SetByteArrayRegion to avoid memory copy
from
>>>>>>>> java
>>>>>>>> to native
>>>>>>>> in setByteArray, Get/ReleasePrimitiveArrayCritical instead
of
>>>>>>>> GetByteArrayElements/ReleaseByteArrayElements to avoid memory
copy.
>>>>>>>>
>>>>>> I just found in OSMemory.c, get/setByteArray do some unnecessary
memory
>>>>>> copies between Java and native.
>>>>>>
>>>>>> In getByteArray, GetByteArrayElements copy data from java to native,
>>>>>> and
>>>>>> then ReleaseByteArrayElements do the reverse, I think using
>>>>>> SetByteArrayRegion is enough.
>>>>>>
>>>> +1 for this fix
>>>>
>>>>
>>>>>> In setByteArray, I found JNI calls
>>>>>> GetPrimitiveArrayCritical/ReleasePrimitiveArrayCritical could get
a
>>>>>> pointer
>>>>>> to the primitive array without any copy, but seems they has some
side
>>>>>> effects (on GC?), I don't have much confidence that they can apply
>>>>>> here. I
>>>>>> hope some one can give some advices about this patch. Thanks.
>>>>>>
>>>>>>
>>> According to HARMONY-1634, ReleasePrimitiveArrayCritical should be called
>>> every time, whether "isCopy" is true or not. I'll update patch soon.
>>>
>>>> As I know, use this GetPrimitiveArrayCritical is something like entering
>>>> the
>>>> "critical region", the spec reads:
>>>>
>>>> After calling GetPrimitiveArrayCritical, the native code should not run
>>>> for
>>>> an extended period of time before it calls ReleasePrimitiveArrayCritical.
>>>> We
>>>> must treat the code inside this pair of functions as running in a
>>>> "critical
>>>> region." Inside a critical region, native code must not call other JNI
>>>> functions, or any system call that may cause the current thread to block
>>>> and
>>>> wait for another Java thread. (For example, the current thread must not
>>>> call
>>>> read on a stream being written by another Java thread.)
>>>>
>>>> thus it may potentially stall GC, as well as other jni critical section
>>>> is
>>> Maybe only critical section on the same primitive array would be blocked?
>>>
>>>> delayed, so this may cause another performance degradation.
>>> In this case, the critical section is very small, it should not be a
>>> problem, I think.
>>>
>>>> So the question is, how key is this improvement,  and what benchmark show
>>>> its improvement? I am not sure, it may be in some environment/situations,
>>>> e.g. in some small heap environment, this modification may be a little
>>>> slower?  We'd better do some more benchmarks?
>>> Yes, I have tested on a internal benchmark, I can't say much details, but
>>> it based on real complicated applications and heavily used
>>> DirectByteBuffer.get/put(). And after this patch, it boost 2-3%.
>>>
>>> It's also possible to write a micro benchmark, but as you said it's hard
>>> to simulate all the environment/situations.
>>>
>>>>
>>>>> The patch passed all luni and nio tests, if no one objected, I'd like
to
>>>>> commit it.
>>>>>
>>>>> --
>>>>> Best Regards,
>>>>> Regis.
>>>>>
>>>>
>>>>
>>>
>> Can any vm gurus help to explain how vm implement
>> Get/ReleasePrimitiveArrayCritical and what does "critical region" here
>> exactly mean? So it would be clear whether Get/ReleasePrimitiveArrayCritical
>> is expensive here.
>> Thanks.
>>
>> --
>> Best Regards,
>> Regis.
>>
> 


-- 
Best Regards,
Regis.

Mime
View raw message