harmony-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Tim Ellison <t.p.elli...@gmail.com>
Subject Re: [jira] Created: (HARMONY-6328) [classlib][nio] optimize SocketChannel.write(ByteBuffer[], int, int) by writev
Date Tue, 15 Sep 2009 08:08:06 GMT
On 15/Sep/2009 04:26, Regis wrote:
> Thanks for helping to review the patch.
> 
> Tim Ellison wrote:
>> On 07/Sep/2009 09:33, Regis wrote:
>>> I managed to implement in this way and submit a patch to JIRA.
>>>
>>> I tried to pass a object array, if it's direct buffer, fill the array
>>> element with buffer directly (heap byte buffer without an array will be
>>> copied to direct buffer first), otherwise, pass byte array. Because we
>>> still need to know the offset of arrays, so I have to pass it to native
>>> code.
>>
>> Thanks Regis.
>>
>> Looking at the patch "HARMONY-6328.v3.diff",
>>
>> (1) This test is checking it is an instance of java.nio.ByteBuffer, but
>> it should be checking for instances of java.nio.DirectByteBuffer, right?
>>
>> isDirectBuffer = (*env)->IsInstanceOf(env, buffer, byteBufferClass);
> 
> It's a little bit trick here. java.nio.DirectByteBuffer is not a
> standard API, so I hesitated to use it here. And the elements of passed
> in "buffers" must be direct buffer or a byte array, so it's safe to
> check it is an instance of java.nio.ByteBuffer.

I see, but it would be clearer if it checked for DirectByteBuffer don't
you think?  I know that is not a public type, but the natives are
certainly not limited to dealing with public elements (e.g. the call you
make earlier to getJavaIoFileDescriptorContentsAsAPointer()).

>> (2) Not sure if any JNI implementations will care, but AIUI you only
>> should call ReleaseByteArrayElements if you got a copy of the array
>> (i.e. as indicated by the GetByteArrayElements).
> 
> I had the puzzle too, and did some searches, I found HARMONY-1634, not
> sure it's just suitable for drlvm, but "The Get.. Release pair can be
> used to prevent GC during the operation" seems reasonable, so I followed
> it.

My point was that, AIUI, the Release should only be called if the Get
returned a copy, and you are not checking whether the Get returned a
copy or not.  In practice, it probably doesn't matter anyway.

Regards,
Tim

>> I didn't study the SocketChannelImpl too closely, but it looks better
>> now :-)  If you agree with (at leat) (1) above then we should apply the
>> patch and do some comparisons!
>>
>> Regards,
>> Tim
>>
> 
> 

Mime
View raw message