You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@harmony.apache.org by Regis <xu...@gmail.com> on 2009/09/03 11:12:17 UTC
Re: [jira] Created: (HARMONY-6328) [classlib][nio] optimize SocketChannel.write(ByteBuffer[],
int, int) by writev
Regis Xu (JIRA) wrote:
> [classlib][nio] optimize SocketChannel.write(ByteBuffer[], int, int) by writev
> ------------------------------------------------------------------------------
>
> Key: HARMONY-6328
> URL: https://issues.apache.org/jira/browse/HARMONY-6328
> Project: Harmony
> Issue Type: Improvement
> Components: Classlib
> Affects Versions: 5.0M11
> Reporter: Regis Xu
>
>
> SocketChannel.write(ByteBuffer[], int, int) can be optimized by using writev, which can write an array of buffer to socket without any stage buffer. If all passed in ByteBuffer is direct buffer, they can be passed to system call directly without any copies.
>
I tested the patch on Windows, Linux and AIX, nio tests are all passed. I know
someone (Oliver, Kevin and Ray) are working on z/OS, I'm not sure whether writev
will work on it, so it would be great if someone can test the patch on z/OS
before committing it.
And also, any comments and suggestions about this improvement are welcome :)
--
Best Regards,
Regis.
Re: [jira] Created: (HARMONY-6328) [classlib][nio] optimize SocketChannel.write(ByteBuffer[],
int, int) by writev
Posted by Tim Ellison <t....@gmail.com>.
On 28/Sep/2009 06:26, Regis wrote:
> It's a little sad it will break VME. I thought two ways to fix this:
>
> 1. add new parameter long[] to writev, which contains native address of
> direct buffer, but the parameters will be a little long.
>
> 2. wrap native address to Long which can be filled in Object[], but this
> may need reflections to get each value of Long.
>
> Both of them are not so good....
I think we should just raise a bug report to fix the VME.
The exception raised was confusing though,
java.net.SocketException: (10038) An operation was attempted on
something that is not a socket.
Perhaps we do need some more error checking in the native code to cover
these cases so it reports an internal error or something. It might also
be worth refactoring the code so that the majority is shared, with just
the OS calls moved out to the platform specific directories.
Regards,
Tim
Re: [jira] Created: (HARMONY-6328) [classlib][nio] optimize SocketChannel.write(ByteBuffer[],
int, int) by writev
Posted by Regis <xu...@gmail.com>.
Tim Ellison wrote:
> On 24/Sep/2009 06:05, Regis wrote:
>> Tim Ellison wrote:
>>> On 18/Sep/2009 08:58, Regis wrote:
>>>> I applied the improvements at r816508 and closed the JIRA.
>>> Great. I assume it is just me, but I'm running with the IBM VME and see
>>> a failure in the new test:
>>>
>>>> java -version
>>> Apache Harmony Launcher : (c) Copyright 1991, 2009 The Apache Software
>>> Foundation or its licensors, as applicable.
>>> java version "1.4.2 subset"
>>> Harmony Virtual Machine Element (2.3)
>>> J9 (2.3)
>>> IBM J9 2.3 Windows XP x86-32 (JIT enabled)
>>> J9VM - 20060727_07300_lHdSMR
>>> JIT - 20060727_1808_r8
>>> GC - 20060724_AA
>>>
>>> produces
>>>
>>> (10038) An operation was attempted on something that is not a socket.
>>>
>>> java.net.SocketException: (10038) An operation was attempted on
>>> something that is not a socket.
>>> at org.apache.harmony.luni.platform.OSNetworkSystem.writev(Native Method)
>>> at
>>> org.apache.harmony.nio.internal.SocketChannelImpl.writevImpl(SocketChannelImpl.java:554)
>>>
>>> at
>>> org.apache.harmony.nio.internal.SocketChannelImpl.write(SocketChannelImpl.java:530)
>>>
>>> at
>>> org.apache.harmony.nio.tests.java.nio.channels.SocketChannelTest.test_writev(SocketChannelTest.java:2673)
>>>
>>> at java.lang.reflect.AccessibleObject.invokeV(AccessibleObject.java:25)
>>>
>>> I'll try to recreate on DRLVM and open a JIRA if it is still failing
>>> for me.
>>>
>>> I have written a number of new tests for this code too, but they'll have
>>> to wait until I get past this one :-)
>>>
>>> Regards,
>>> Tim
>>>
>>>
>>>
>> I found the failures only happened when buffers contained direct buffer,
>> after further investigation, (*env)->GetDirectBufferAddress always
>> return 0 in IBM VME. Maybe IBM VME doesn't support this JNI call or
>> doesn't recognize Harmony direct buffer implementation?
>
> Ah, I expect it is that the VME doesn't work with the Harmony NIO
> implementation yet. Things were working ok with the DRLVM so I went
> ahead and committed my enhanced tests. I'm happy to say they all worked
> perfectly with your optimizations too, good work! :-)
>
> Regards,
> Tim
>
It's a little sad it will break VME. I thought two ways to fix this:
1. add new parameter long[] to writev, which contains native address of direct
buffer, but the parameters will be a little long.
2. wrap native address to Long which can be filled in Object[], but this may
need reflections to get each value of Long.
Both of them are not so good....
--
Best Regards,
Regis.
Re: [jira] Created: (HARMONY-6328) [classlib][nio] optimize SocketChannel.write(ByteBuffer[],
int, int) by writev
Posted by Tim Ellison <t....@gmail.com>.
On 24/Sep/2009 06:05, Regis wrote:
> Tim Ellison wrote:
>> On 18/Sep/2009 08:58, Regis wrote:
>>> I applied the improvements at r816508 and closed the JIRA.
>>
>> Great. I assume it is just me, but I'm running with the IBM VME and see
>> a failure in the new test:
>>
>>> java -version
>> Apache Harmony Launcher : (c) Copyright 1991, 2009 The Apache Software
>> Foundation or its licensors, as applicable.
>> java version "1.4.2 subset"
>> Harmony Virtual Machine Element (2.3)
>> J9 (2.3)
>> IBM J9 2.3 Windows XP x86-32 (JIT enabled)
>> J9VM - 20060727_07300_lHdSMR
>> JIT - 20060727_1808_r8
>> GC - 20060724_AA
>>
>> produces
>>
>> (10038) An operation was attempted on something that is not a socket.
>>
>> java.net.SocketException: (10038) An operation was attempted on
>> something that is not a socket.
>> at org.apache.harmony.luni.platform.OSNetworkSystem.writev(Native Method)
>> at
>> org.apache.harmony.nio.internal.SocketChannelImpl.writevImpl(SocketChannelImpl.java:554)
>>
>> at
>> org.apache.harmony.nio.internal.SocketChannelImpl.write(SocketChannelImpl.java:530)
>>
>> at
>> org.apache.harmony.nio.tests.java.nio.channels.SocketChannelTest.test_writev(SocketChannelTest.java:2673)
>>
>> at java.lang.reflect.AccessibleObject.invokeV(AccessibleObject.java:25)
>>
>> I'll try to recreate on DRLVM and open a JIRA if it is still failing
>> for me.
>>
>> I have written a number of new tests for this code too, but they'll have
>> to wait until I get past this one :-)
>>
>> Regards,
>> Tim
>>
>>
>>
>
> I found the failures only happened when buffers contained direct buffer,
> after further investigation, (*env)->GetDirectBufferAddress always
> return 0 in IBM VME. Maybe IBM VME doesn't support this JNI call or
> doesn't recognize Harmony direct buffer implementation?
Ah, I expect it is that the VME doesn't work with the Harmony NIO
implementation yet. Things were working ok with the DRLVM so I went
ahead and committed my enhanced tests. I'm happy to say they all worked
perfectly with your optimizations too, good work! :-)
Regards,
Tim
Re: [jira] Created: (HARMONY-6328) [classlib][nio] optimize SocketChannel.write(ByteBuffer[],
int, int) by writev
Posted by Regis <xu...@gmail.com>.
Tim Ellison wrote:
> On 18/Sep/2009 08:58, Regis wrote:
>> I applied the improvements at r816508 and closed the JIRA.
>
> Great. I assume it is just me, but I'm running with the IBM VME and see
> a failure in the new test:
>
>> java -version
> Apache Harmony Launcher : (c) Copyright 1991, 2009 The Apache Software
> Foundation or its licensors, as applicable.
> java version "1.4.2 subset"
> Harmony Virtual Machine Element (2.3)
> J9 (2.3)
> IBM J9 2.3 Windows XP x86-32 (JIT enabled)
> J9VM - 20060727_07300_lHdSMR
> JIT - 20060727_1808_r8
> GC - 20060724_AA
>
> produces
>
> (10038) An operation was attempted on something that is not a socket.
>
> java.net.SocketException: (10038) An operation was attempted on
> something that is not a socket.
> at org.apache.harmony.luni.platform.OSNetworkSystem.writev(Native Method)
> at
> org.apache.harmony.nio.internal.SocketChannelImpl.writevImpl(SocketChannelImpl.java:554)
> at
> org.apache.harmony.nio.internal.SocketChannelImpl.write(SocketChannelImpl.java:530)
> at
> org.apache.harmony.nio.tests.java.nio.channels.SocketChannelTest.test_writev(SocketChannelTest.java:2673)
> at java.lang.reflect.AccessibleObject.invokeV(AccessibleObject.java:25)
>
> I'll try to recreate on DRLVM and open a JIRA if it is still failing for me.
>
> I have written a number of new tests for this code too, but they'll have
> to wait until I get past this one :-)
>
> Regards,
> Tim
>
>
>
I found the failures only happened when buffers contained direct buffer, after
further investigation, (*env)->GetDirectBufferAddress always return 0 in IBM
VME. Maybe IBM VME doesn't support this JNI call or doesn't recognize Harmony
direct buffer implementation?
--
Best Regards,
Regis.
Re: [jira] Created: (HARMONY-6328) [classlib][nio] optimize SocketChannel.write(ByteBuffer[], int, int) by writev
Posted by Mark Hindess <ma...@googlemail.com>.
In message <4A...@gmail.com>, Tim Ellison writes:
>
> On 18/Sep/2009 08:58, Regis wrote:
> > I applied the improvements at r816508 and closed the JIRA.
>
> Great. I assume it is just me, but I'm running with the IBM VME and see
> a failure in the new test:
>
> > java -version
> Apache Harmony Launcher : (c) Copyright 1991, 2009 The Apache Software
> Foundation or its licensors, as applicable.
> java version "1.4.2 subset"
> Harmony Virtual Machine Element (2.3)
> J9 (2.3)
> IBM J9 2.3 Windows XP x86-32 (JIT enabled)
> J9VM - 20060727_07300_lHdSMR
> JIT - 20060727_1808_r8
> GC - 20060724_AA
>
> produces
>
> (10038) An operation was attempted on something that is not a socket.
>
> java.net.SocketException: (10038) An operation was attempted on
> something that is not a socket.
> at org.apache.harmony.luni.platform.OSNetworkSystem.writev(Native Method)
> at
> org.apache.harmony.nio.internal.SocketChannelImpl.writevImpl(SocketChannelImp
> l.java:554)
> at
> org.apache.harmony.nio.internal.SocketChannelImpl.write(SocketChannelImpl.jav
> a:530)
> at
> org.apache.harmony.nio.tests.java.nio.channels.SocketChannelTest.test_writev(
> SocketChannelTest.java:2673)
> at java.lang.reflect.AccessibleObject.invokeV(AccessibleObject.java:25)
>
> I'll try to recreate on DRLVM and open a JIRA if it is still failing for me.
Works for me on DRLVM but it hangs with an IBM VME (since r816508 worked
before that). It could be an USR1/GC interrupt preventing a hang on drlvm.
> I have written a number of new tests for this code too, but they'll have
> to wait until I get past this one :-)
Excellent.
-Mark.
Re: [jira] Created: (HARMONY-6328) [classlib][nio] optimize SocketChannel.write(ByteBuffer[],
int, int) by writev
Posted by Tim Ellison <t....@gmail.com>.
On 18/Sep/2009 08:58, Regis wrote:
> I applied the improvements at r816508 and closed the JIRA.
Great. I assume it is just me, but I'm running with the IBM VME and see
a failure in the new test:
> java -version
Apache Harmony Launcher : (c) Copyright 1991, 2009 The Apache Software
Foundation or its licensors, as applicable.
java version "1.4.2 subset"
Harmony Virtual Machine Element (2.3)
J9 (2.3)
IBM J9 2.3 Windows XP x86-32 (JIT enabled)
J9VM - 20060727_07300_lHdSMR
JIT - 20060727_1808_r8
GC - 20060724_AA
produces
(10038) An operation was attempted on something that is not a socket.
java.net.SocketException: (10038) An operation was attempted on
something that is not a socket.
at org.apache.harmony.luni.platform.OSNetworkSystem.writev(Native Method)
at
org.apache.harmony.nio.internal.SocketChannelImpl.writevImpl(SocketChannelImpl.java:554)
at
org.apache.harmony.nio.internal.SocketChannelImpl.write(SocketChannelImpl.java:530)
at
org.apache.harmony.nio.tests.java.nio.channels.SocketChannelTest.test_writev(SocketChannelTest.java:2673)
at java.lang.reflect.AccessibleObject.invokeV(AccessibleObject.java:25)
I'll try to recreate on DRLVM and open a JIRA if it is still failing for me.
I have written a number of new tests for this code too, but they'll have
to wait until I get past this one :-)
Regards,
Tim
Re: [jira] Created: (HARMONY-6328) [classlib][nio] optimize SocketChannel.write(ByteBuffer[],
int, int) by writev
Posted by Regis <xu...@gmail.com>.
Tim Ellison wrote:
> On 15/Sep/2009 09:51, Regis wrote:
>> Tim Ellison wrote:
>>> On 15/Sep/2009 04:26, Regis wrote:
>>>> Tim Ellison wrote:
>>>>> (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.
>> AIUI, If Get pinned java heap, the pinned array will be locked to
>> prevent GC collect or move it, Release will unlock it. So I think
>> Release should be called anyway, like we did in HARMONY-1634.
>
> The Get* function will tell you if it has been pinned or copied via the
> isCopy argument. However, as I read the spec [1] I think you are right
> that Release* should/could be called every time (which is likely a
> trivial operation for pinned arrays anyway).
>
> I was looking at the Essential JNI book [2] reference page that says,
> "This value [isCopy] should be tested to determine if a call to
> Release<pType>ArrayElements needs to be called." (p.378)
>
> I normally wrap the Release* function call in an if(isCopy == JNI_TRUE)
> conditional, but maybe more by habit than necessity.
>
> I agree it is different for GetPrimitiveArrayCritical (which was the
> subject of HARMONY-1634).
>
> [1]
> http://java.sun.com/j2se/1.5.0/docs/guide/jni/spec/functions.html#wp17382
> [2] http://www.amazon.com/Essential-Jni-Java-Native-Interface/dp/0136798950
>
> Regards,
> Tim
>
I applied the improvements at r816508 and closed the JIRA.
--
Best Regards,
Regis.
Re: [jira] Created: (HARMONY-6328) [classlib][nio] optimize SocketChannel.write(ByteBuffer[],
int, int) by writev
Posted by Tim Ellison <t....@gmail.com>.
On 15/Sep/2009 09:51, Regis wrote:
> Tim Ellison wrote:
>> On 15/Sep/2009 04:26, Regis wrote:
>>> Tim Ellison wrote:
>>>> (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.
>
> AIUI, If Get pinned java heap, the pinned array will be locked to
> prevent GC collect or move it, Release will unlock it. So I think
> Release should be called anyway, like we did in HARMONY-1634.
The Get* function will tell you if it has been pinned or copied via the
isCopy argument. However, as I read the spec [1] I think you are right
that Release* should/could be called every time (which is likely a
trivial operation for pinned arrays anyway).
I was looking at the Essential JNI book [2] reference page that says,
"This value [isCopy] should be tested to determine if a call to
Release<pType>ArrayElements needs to be called." (p.378)
I normally wrap the Release* function call in an if(isCopy == JNI_TRUE)
conditional, but maybe more by habit than necessity.
I agree it is different for GetPrimitiveArrayCritical (which was the
subject of HARMONY-1634).
[1]
http://java.sun.com/j2se/1.5.0/docs/guide/jni/spec/functions.html#wp17382
[2] http://www.amazon.com/Essential-Jni-Java-Native-Interface/dp/0136798950
Regards,
Tim
Re: [jira] Created: (HARMONY-6328) [classlib][nio] optimize SocketChannel.write(ByteBuffer[],
int, int) by writev
Posted by Regis <xu...@gmail.com>.
Tim Ellison wrote:
> 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()).
>
Agreed, I will change it.
>>> (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.
AIUI, If Get pinned java heap, the pinned array will be locked to prevent GC
collect or move it, Release will unlock it. So I think Release should be called
anyway, like we did in HARMONY-1634.
>
> 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
>>>
>>
>
--
Best Regards,
Regis.
Re: [jira] Created: (HARMONY-6328) [classlib][nio] optimize SocketChannel.write(ByteBuffer[],
int, int) by writev
Posted by Tim Ellison <t....@gmail.com>.
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
>>
>
>
Re: [jira] Created: (HARMONY-6328) [classlib][nio] optimize SocketChannel.write(ByteBuffer[],
int, int) by writev
Posted by Regis <xu...@gmail.com>.
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.
>
> (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.
>
>
> 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
>
--
Best Regards,
Regis.
Re: [jira] Created: (HARMONY-6328) [classlib][nio] optimize SocketChannel.write(ByteBuffer[],
int, int) by writev
Posted by Tim Ellison <t....@gmail.com>.
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);
(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 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
Re: [jira] Created: (HARMONY-6328) [classlib][nio] optimize SocketChannel.write(ByteBuffer[],
int, int) by writev
Posted by Tim Ellison <t....@gmail.com>.
On 07/Sep/2009 09:33, Regis wrote:
> Tim Ellison wrote:
>> On 03/Sep/2009 12:29, Regis wrote:
>>> Tim Ellison wrote:
>>>> A minor comment first :-) Please check return values for :
>>>>
>>>> + vect = (struct iovec*) hymem_allocate_memory(sizeof(struct iovec) *
>>>> length);
>>>> +
>>>> + message = (*env)->GetPrimitiveArrayCritical(env, addrs,
>>>> &isCopyMessage);
>>>> + cts = (*env)->GetPrimitiveArrayCritical(env, counts, &isCopyCts);
>>> Yes, will add it, thanks.
>>>
>>>>
>>>> A further enhancement is to have two versions of primitives, one that
>>>> deals with direct buffers and one that deals with java heap buffers, so
>>>> that there is (potentially) no data copying required for the java-heap
>>>> buffers version that you have got today:
>>> One case stop me to do that is a buffer array contains direct and java
>>> heap buffers both. In this case, either copying java heap buffres to
>>> direct buffers or passing the whole array to native directly.
>>
>> Yes, good point.
>>
>>> If pass the array to native directly, it need to call isDirect and
>>> AddressUtil.getDirectBufferAddress in native code, as I understand, call
>>> java methods from native side is like reflection, even more expensive,
>>> so I chose copying all java heap buffers to direct buffers, at least it
>>> fast when most buffers are direct and size of java heap buffers are not
>>> too big.
>>
>> No need to use call back into Java code to do this, there are already
>> JNI functions you can use:
>>
>> jboolean IsInstanceOf(JNIEnv *env, jobject obj, jclass clazz);
>> void* GetDirectBufferAddress(JNIEnv* env, jobject buf);
>>
>> See the spec at
>> http://java.sun.com/j2se/1.5.0/docs/guide/jni/spec/functions.html
>>
>> However, you cannot always get the backing array from a ByteBuffer, so
>> you still have three cases to deal with:
>> 1. the direct byte buffer (by address)
>> 2. the heap byte buffer with backing array (may be pinned/copied)
>> 3. the heap byte buffer without an array (must be copied)
>>
>> For case (3) likely it is best to copy into a direct byte buffer to
>> ensure it does not get copied again, so you end up passing only types
>> (1) and (2) into the native.
>>
>>> One possible solution I can think is adding a new native call
>>>
>>> writevDirect(JNIEnv *env, jobject thiz, jobject fd, jobject bufferArray,
>>> jobject addrs, jobject counts, jint length)
>>>
>>> passing buffer array and native addresses of direct buffers together, if
>>> element of addrs is 0, the corresponding buffer is not direct. Well,
>>> this method is a little bit hard to understand, it has 7 parameters....
>>
>> I agree. Maybe the best way is to pass them in as jobjects and let the
>> native code sort out the direct / heap buffers -- using the address
>> directly and getting access to the bytes respectively.
>>
>
> 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.
>
> More test cases and documents are coming...
Wow, quick work! I'll take a look.
Regards,
Tim
Re: [jira] Created: (HARMONY-6328) [classlib][nio] optimize SocketChannel.write(ByteBuffer[],
int, int) by writev
Posted by Regis <xu...@gmail.com>.
Tim Ellison wrote:
> On 03/Sep/2009 12:29, Regis wrote:
>> Tim Ellison wrote:
>>> A minor comment first :-) Please check return values for :
>>>
>>> + vect = (struct iovec*) hymem_allocate_memory(sizeof(struct iovec) *
>>> length);
>>> +
>>> + message = (*env)->GetPrimitiveArrayCritical(env, addrs,
>>> &isCopyMessage);
>>> + cts = (*env)->GetPrimitiveArrayCritical(env, counts, &isCopyCts);
>> Yes, will add it, thanks.
>>
>>>
>>> A further enhancement is to have two versions of primitives, one that
>>> deals with direct buffers and one that deals with java heap buffers, so
>>> that there is (potentially) no data copying required for the java-heap
>>> buffers version that you have got today:
>> One case stop me to do that is a buffer array contains direct and java
>> heap buffers both. In this case, either copying java heap buffres to
>> direct buffers or passing the whole array to native directly.
>
> Yes, good point.
>
>> If pass the array to native directly, it need to call isDirect and
>> AddressUtil.getDirectBufferAddress in native code, as I understand, call
>> java methods from native side is like reflection, even more expensive,
>> so I chose copying all java heap buffers to direct buffers, at least it
>> fast when most buffers are direct and size of java heap buffers are not
>> too big.
>
> No need to use call back into Java code to do this, there are already
> JNI functions you can use:
>
> jboolean IsInstanceOf(JNIEnv *env, jobject obj, jclass clazz);
> void* GetDirectBufferAddress(JNIEnv* env, jobject buf);
>
> See the spec at
> http://java.sun.com/j2se/1.5.0/docs/guide/jni/spec/functions.html
>
> However, you cannot always get the backing array from a ByteBuffer, so
> you still have three cases to deal with:
> 1. the direct byte buffer (by address)
> 2. the heap byte buffer with backing array (may be pinned/copied)
> 3. the heap byte buffer without an array (must be copied)
>
> For case (3) likely it is best to copy into a direct byte buffer to
> ensure it does not get copied again, so you end up passing only types
> (1) and (2) into the native.
>
>> One possible solution I can think is adding a new native call
>>
>> writevDirect(JNIEnv *env, jobject thiz, jobject fd, jobject bufferArray,
>> jobject addrs, jobject counts, jint length)
>>
>> passing buffer array and native addresses of direct buffers together, if
>> element of addrs is 0, the corresponding buffer is not direct. Well,
>> this method is a little bit hard to understand, it has 7 parameters....
>
> I agree. Maybe the best way is to pass them in as jobjects and let the
> native code sort out the direct / heap buffers -- using the address
> directly and getting access to the bytes respectively.
>
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.
More test cases and documents are coming...
--
Best Regards,
Regis.
Re: [jira] Created: (HARMONY-6328) [classlib][nio] optimize SocketChannel.write(ByteBuffer[],
int, int) by writev
Posted by Tim Ellison <t....@gmail.com>.
On 03/Sep/2009 12:29, Regis wrote:
> Tim Ellison wrote:
>> A minor comment first :-) Please check return values for :
>>
>> + vect = (struct iovec*) hymem_allocate_memory(sizeof(struct iovec) *
>> length);
>> +
>> + message = (*env)->GetPrimitiveArrayCritical(env, addrs,
>> &isCopyMessage);
>> + cts = (*env)->GetPrimitiveArrayCritical(env, counts, &isCopyCts);
>
> Yes, will add it, thanks.
>
>>
>>
>> A further enhancement is to have two versions of primitives, one that
>> deals with direct buffers and one that deals with java heap buffers, so
>> that there is (potentially) no data copying required for the java-heap
>> buffers version that you have got today:
>
> One case stop me to do that is a buffer array contains direct and java
> heap buffers both. In this case, either copying java heap buffres to
> direct buffers or passing the whole array to native directly.
Yes, good point.
> If pass the array to native directly, it need to call isDirect and
> AddressUtil.getDirectBufferAddress in native code, as I understand, call
> java methods from native side is like reflection, even more expensive,
> so I chose copying all java heap buffers to direct buffers, at least it
> fast when most buffers are direct and size of java heap buffers are not
> too big.
No need to use call back into Java code to do this, there are already
JNI functions you can use:
jboolean IsInstanceOf(JNIEnv *env, jobject obj, jclass clazz);
void* GetDirectBufferAddress(JNIEnv* env, jobject buf);
See the spec at
http://java.sun.com/j2se/1.5.0/docs/guide/jni/spec/functions.html
However, you cannot always get the backing array from a ByteBuffer, so
you still have three cases to deal with:
1. the direct byte buffer (by address)
2. the heap byte buffer with backing array (may be pinned/copied)
3. the heap byte buffer without an array (must be copied)
For case (3) likely it is best to copy into a direct byte buffer to
ensure it does not get copied again, so you end up passing only types
(1) and (2) into the native.
> One possible solution I can think is adding a new native call
>
> writevDirect(JNIEnv *env, jobject thiz, jobject fd, jobject bufferArray,
> jobject addrs, jobject counts, jint length)
>
> passing buffer array and native addresses of direct buffers together, if
> element of addrs is 0, the corresponding buffer is not direct. Well,
> this method is a little bit hard to understand, it has 7 parameters....
I agree. Maybe the best way is to pass them in as jobjects and let the
native code sort out the direct / heap buffers -- using the address
directly and getting access to the bytes respectively.
>> + if (!buffer.isDirect()) {
>> + src[i] = ByteBuffer.allocateDirect(buffer.remaining());
>> + int oldPosition = buffer.position();
>> + src[i].put(buffer);
>>
>> some VMs will pin the java heap memory accessed in JNI
>> (GetByteArrayElements), so if you can get hold of the ByteBuffer#array()
>> you can pass a set of pointers to the backing arrays and maybe send the
>> data directly.
>>
>> Looking forward to seeing the readv impl too ;-)
>
> Sure, it's on my TODO list :)
Cool.
Regards,
Tim
Re: [jira] Created: (HARMONY-6328) [classlib][nio] optimize SocketChannel.write(ByteBuffer[],
int, int) by writev
Posted by Regis <xu...@gmail.com>.
Ray Chen wrote:
> Hi Reigs,
> I have tested your patch on z/OS, it works OK.
>
> Greate job.
>
Good news, Thanks Ray!!
--
Best Regards,
Regis.
Re: [jira] Created: (HARMONY-6328) [classlib][nio] optimize
SocketChannel.write(ByteBuffer[], int, int) by writev
Posted by Ray Chen <cl...@gmail.com>.
Hi Reigs,
I have tested your patch on z/OS, it works OK.
Greate job.
On Thu, Sep 3, 2009 at 7:29 PM, Regis <xu...@gmail.com> wrote:
> Tim Ellison wrote:
>
>> A minor comment first :-) Please check return values for :
>>
>> + vect = (struct iovec*) hymem_allocate_memory(sizeof(struct iovec) *
>> length);
>> +
>> + message = (*env)->GetPrimitiveArrayCritical(env, addrs, &isCopyMessage);
>> + cts = (*env)->GetPrimitiveArrayCritical(env, counts, &isCopyCts);
>>
>
> Yes, will add it, thanks.
>
>
>>
>> A further enhancement is to have two versions of primitives, one that
>> deals with direct buffers and one that deals with java heap buffers, so
>> that there is (potentially) no data copying required for the java-heap
>> buffers version that you have got today:
>>
>
> One case stop me to do that is a buffer array contains direct and java heap
> buffers both. In this case, either copying java heap buffres to direct
> buffers or passing the whole array to native directly.
>
> If pass the array to native directly, it need to call isDirect and
> AddressUtil.getDirectBufferAddress in native code, as I understand, call
> java methods from native side is like reflection, even more expensive, so I
> chose copying all java heap buffers to direct buffers, at least it fast when
> most buffers are direct and size of java heap buffers are not too big.
>
> One possible solution I can think is adding a new native call
>
> writevDirect(JNIEnv *env, jobject thiz, jobject fd, jobject bufferArray,
> jobject addrs, jobject counts, jint length)
>
> passing buffer array and native addresses of direct buffers together, if
> element of addrs is 0, the corresponding buffer is not direct. Well, this
> method is a little bit hard to understand, it has 7 parameters....
>
>
>> + if (!buffer.isDirect()) {
>> + src[i] = ByteBuffer.allocateDirect(buffer.remaining());
>> + int oldPosition = buffer.position();
>> + src[i].put(buffer);
>>
>> some VMs will pin the java heap memory accessed in JNI
>> (GetByteArrayElements), so if you can get hold of the ByteBuffer#array()
>> you can pass a set of pointers to the backing arrays and maybe send the
>> data directly.
>>
>> Looking forward to seeing the readv impl too ;-)
>>
>
> Sure, it's on my TODO list :)
>
>
>> Regards,
>> Tim
>>
>>
>>
>
> --
> Best Regards,
> Regis.
>
--
Best Regards,
Ray Chen
Re: [jira] Created: (HARMONY-6328) [classlib][nio] optimize SocketChannel.write(ByteBuffer[],
int, int) by writev
Posted by Regis <xu...@gmail.com>.
Tim Ellison wrote:
> A minor comment first :-) Please check return values for :
>
> + vect = (struct iovec*) hymem_allocate_memory(sizeof(struct iovec) *
> length);
> +
> + message = (*env)->GetPrimitiveArrayCritical(env, addrs, &isCopyMessage);
> + cts = (*env)->GetPrimitiveArrayCritical(env, counts, &isCopyCts);
Yes, will add it, thanks.
>
>
> A further enhancement is to have two versions of primitives, one that
> deals with direct buffers and one that deals with java heap buffers, so
> that there is (potentially) no data copying required for the java-heap
> buffers version that you have got today:
One case stop me to do that is a buffer array contains direct and java heap
buffers both. In this case, either copying java heap buffres to direct buffers
or passing the whole array to native directly.
If pass the array to native directly, it need to call isDirect and
AddressUtil.getDirectBufferAddress in native code, as I understand, call java
methods from native side is like reflection, even more expensive, so I chose
copying all java heap buffers to direct buffers, at least it fast when most
buffers are direct and size of java heap buffers are not too big.
One possible solution I can think is adding a new native call
writevDirect(JNIEnv *env, jobject thiz, jobject fd, jobject bufferArray, jobject
addrs, jobject counts, jint length)
passing buffer array and native addresses of direct buffers together, if element
of addrs is 0, the corresponding buffer is not direct. Well, this method is a
little bit hard to understand, it has 7 parameters....
>
> + if (!buffer.isDirect()) {
> + src[i] = ByteBuffer.allocateDirect(buffer.remaining());
> + int oldPosition = buffer.position();
> + src[i].put(buffer);
>
> some VMs will pin the java heap memory accessed in JNI
> (GetByteArrayElements), so if you can get hold of the ByteBuffer#array()
> you can pass a set of pointers to the backing arrays and maybe send the
> data directly.
>
> Looking forward to seeing the readv impl too ;-)
Sure, it's on my TODO list :)
>
> Regards,
> Tim
>
>
--
Best Regards,
Regis.
Re: [jira] Created: (HARMONY-6328) [classlib][nio] optimize SocketChannel.write(ByteBuffer[],
int, int) by writev
Posted by Tim Ellison <t....@gmail.com>.
On 03/Sep/2009 10:12, Regis wrote:
> Regis Xu (JIRA) wrote:
>> [classlib][nio] optimize SocketChannel.write(ByteBuffer[], int, int)
>> by writev
>> ------------------------------------------------------------------------------
>>
>>
>> Key: HARMONY-6328
>> URL: https://issues.apache.org/jira/browse/HARMONY-6328
>> Project: Harmony
>> Issue Type: Improvement
>> Components: Classlib
>> Affects Versions: 5.0M11
>> Reporter: Regis Xu
>>
>>
>> SocketChannel.write(ByteBuffer[], int, int) can be optimized by using
>> writev, which can write an array of buffer to socket without any stage
>> buffer. If all passed in ByteBuffer is direct buffer, they can be
>> passed to system call directly without any copies.
>>
>
> I tested the patch on Windows, Linux and AIX, nio tests are all passed.
> I know someone (Oliver, Kevin and Ray) are working on z/OS, I'm not sure
> whether writev will work on it, so it would be great if someone can
> test the patch on z/OS before committing it.
>
> And also, any comments and suggestions about this improvement are
> welcome :)
I added a comment to JIRA a moment ago, which says...
Good to see this enhancement!
A minor comment first :-) Please check return values for :
+ vect = (struct iovec*) hymem_allocate_memory(sizeof(struct iovec) *
length);
+
+ message = (*env)->GetPrimitiveArrayCritical(env, addrs, &isCopyMessage);
+ cts = (*env)->GetPrimitiveArrayCritical(env, counts, &isCopyCts);
A further enhancement is to have two versions of primitives, one that
deals with direct buffers and one that deals with java heap buffers, so
that there is (potentially) no data copying required for the java-heap
buffers version that you have got today:
+ if (!buffer.isDirect()) {
+ src[i] = ByteBuffer.allocateDirect(buffer.remaining());
+ int oldPosition = buffer.position();
+ src[i].put(buffer);
some VMs will pin the java heap memory accessed in JNI
(GetByteArrayElements), so if you can get hold of the ByteBuffer#array()
you can pass a set of pointers to the backing arrays and maybe send the
data directly.
Looking forward to seeing the readv impl too ;-)
Regards,
Tim