Best practices to work on arrays via JNI from a few threads

[David Holmes]

Clarification ...

On 8/10/2021 9:57 am, David Holmes wrote:
> Hi Sergey,
> 
> On 8/10/2021 7:39 am, Sergey Bylokhov wrote:
>> Hello, hotspot team.
>>
>> I would like to clarify what is the current best practices to work 
>> with arrays from the JNI.
>>
>> Currently I am working on the performance improvement in the color 
>> conversion area in the sun.java2d.cmm package. The logic there is 
>> simple pass the array of data(can be one pixel or huge image) to the 
>> native via JNI, then call some littlecms code(this is third party 
>> library) and save result back. For now that code uses 
>> GetByteArrayElements/ReleaseByteArrayElements. I have found that in 
>> some cases we spent most of the time(60%) copying data. So I have a 
>> few ideas on how to improve the code:
>>   - Use the GetPrimitiveArrayCritical/ReleasePrimitiveArrayCritical - 
>> it will increase the performance of the single threaded code, but may 
>> have some contention issue
>>   - Split the work across the threads
>>
>> Do I understand correctly that I cannot use both improvements together?
>> The Critical API if not in the current implementation, but 
>> theoretically may return a copy of array, so if one thread will get a 
>> copy it will overwrite the whole array and delete the changes of the 
>> other threads.
> 
> The final parameter to GetPrimitiveArrayCritical is "jboolean *isCopy", 
> so you can pass the address of a variable there and know whether you 
> have a copy or not, and then decide how to proceed from there. Hotspot 
> will not make a copy but will use either object-pinning (for GC's that 
> do that - only Shenandoah) or the GCLocker. Deferring GC may cause you 
> other problems so you have to evaluate how long you will need to process 
> the array data in the worst case. You can process the raw array in 
> multiple threads, but then you need to synchronize those threads to know 
> when the array can be released again.

I was thinking of a model where one thread calls 
GetPrimitiveArrayCritical, verifies it has a non-copy, and then shares 
it with other threads. You could of course have each thread call 
GetPrimitiveArrayCritical and just work on its own region of the array, 
but if you wanted to guard against a theoretical VM that might return a 
copy then the fallback code would be much more complicated.

David
-----

>> Is my understanding correct that since there is no 
>> GetPrimitiveArrayRegionCritical API, I need to use 
>> GetByteArrayRegion/SetByteArrayRegion for each slice in each thread?
> 
> If you want to parallelise the copying overhead then yes you need to use 
> the region API in each thread.
> 
>> Probably we have some other way to do this job without coping the data 
>> fourth and back? Is it possible to create a view of the array and then 
>> lock only that slice via GetPrimitiveArrayCritical?
> 
> An array is an array, it can't be partitioned as you describe.
> 
>> Or the Critical API became obsolete due to many issues described here?:
>>   * https://bugs.openjdk.java.net/browse/JDK-8199919?
>>   * 
>> http://mail.openjdk.java.net/pipermail/core-libs-dev/2018-March/052153.html 
>>
>>
>> Thank you for any suggestions.
>>
>> PS: I have read the long similar thread "GetPrimitiveArrayCritical vs 
>> GetByteArrayRegion":
>> http://mail.openjdk.java.net/pipermail/core-libs-dev/2018-March/051898.html 
>>
>> Where Martin suggested to add the "GetPrimitiveArrayRegionCritical", 
>> as far as understand we do not have such plans?
> 
> None of the RFE's from that discussion were picked up.
> 
> Cheers,
> David