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 Mon, 03 Aug 2009 06:29:21 GMT
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