harmony-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Pavel Pervov <pmcfi...@gmail.com>
Subject Re: call for review: (HARMONY-6257) [classlib][luni] - Optimize OSMemory.get/setByteArray
Date Mon, 03 Aug 2009 22:30:50 GMT
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.

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

Mime
View raw message