You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@thrift.apache.org by "ryan rawson (JIRA)" <ji...@apache.org> on 2011/02/09 01:18:57 UTC
[jira] Created: (THRIFT-1058) All ByteBuffers the client uses
should be slice()ed
All ByteBuffers the client uses should be slice()ed
---------------------------------------------------
Key: THRIFT-1058
URL: https://issues.apache.org/jira/browse/THRIFT-1058
Project: Thrift
Issue Type: Bug
Affects Versions: 0.6
Reporter: ryan rawson
Right now the code does something like yay so:
return ByteBuffer.wrap(array, pos, len);
The only problem is that .wrap(byte[],int,int) returns a ByteBuffer with position equal to pos and limit set to pos+len. Which means users cant used 'rewind' or position(0).
It would be cleaner if the contract was "ByteBuffers will have position=0, limit=how much data you have".
The way to accomplish this would be to:
return ByteBuffer.wrap(array, pos, len).slice();
In exchange for 1 extra object alloc (which the GC can clean up quick) we get the benefit of a easier to use BB interface.
--
This message is automatically generated by JIRA.
-
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] Commented: (THRIFT-1058) All ByteBuffers the client uses
should be slice()ed
Posted by "Bryan Duxbury (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-1058?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12992574#comment-12992574 ]
Bryan Duxbury commented on THRIFT-1058:
---------------------------------------
I did a micro benchmark and found that calling slice() takes pretty consistently double the time as just doing wrap(). I don't think the usability benefit (which is, to me, still somewhat dubious) is worth any performance hit.
> All ByteBuffers the client uses should be slice()ed
> ---------------------------------------------------
>
> Key: THRIFT-1058
> URL: https://issues.apache.org/jira/browse/THRIFT-1058
> Project: Thrift
> Issue Type: Bug
> Affects Versions: 0.6
> Reporter: ryan rawson
>
> Right now the code does something like yay so:
> return ByteBuffer.wrap(array, pos, len);
> The only problem is that .wrap(byte[],int,int) returns a ByteBuffer with position equal to pos and limit set to pos+len. Which means users cant used 'rewind' or position(0).
> It would be cleaner if the contract was "ByteBuffers will have position=0, limit=how much data you have".
> The way to accomplish this would be to:
> return ByteBuffer.wrap(array, pos, len).slice();
> In exchange for 1 extra object alloc (which the GC can clean up quick) we get the benefit of a easier to use BB interface.
--
This message is automatically generated by JIRA.
-
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Updated] (THRIFT-1058) All ByteBuffers the client uses
should be slice()ed
Posted by "Jeff Whiting (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-1058?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Jeff Whiting updated THRIFT-1058:
---------------------------------
Attachment: ByteBufferTest.java
We have been running into this problem so I wanted to verify benchmark because it would be better to have the sliced buffer.
My results don't seem to jive:
Iterations: 100000000
Wrap Time: 246ms
Slice Time: 275ms
Wrap & Slice Time: 309ms
javac 1.6.0_24
Java(TM) SE Runtime Environment (build 1.6.0_24-b07-334-10M3326)
Java HotSpot(TM) 64-Bit Server VM (build 19.1-b02-334, mixed mode)
So according to my benchmark there isn't much overhead to do the slice rather than the wrap. Making the change to slice would make it so the ByteBuffer ONLY gets access to its data which provides good encapsulation. Additionally it would improve the client code and allows the ByteBuffer to be reused.
I'm including the benchmark source code so that way if I'm overlooking something you can let me know.
> All ByteBuffers the client uses should be slice()ed
> ---------------------------------------------------
>
> Key: THRIFT-1058
> URL: https://issues.apache.org/jira/browse/THRIFT-1058
> Project: Thrift
> Issue Type: Bug
> Affects Versions: 0.6
> Reporter: ryan rawson
> Attachments: ByteBufferTest.java
>
>
> Right now the code does something like yay so:
> return ByteBuffer.wrap(array, pos, len);
> The only problem is that .wrap(byte[],int,int) returns a ByteBuffer with position equal to pos and limit set to pos+len. Which means users cant used 'rewind' or position(0).
> It would be cleaner if the contract was "ByteBuffers will have position=0, limit=how much data you have".
> The way to accomplish this would be to:
> return ByteBuffer.wrap(array, pos, len).slice();
> In exchange for 1 extra object alloc (which the GC can clean up quick) we get the benefit of a easier to use BB interface.
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (THRIFT-1058) All ByteBuffers the client uses
should be slice()ed
Posted by "Nathaniel Cook (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-1058?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13035589#comment-13035589 ]
Nathaniel Cook commented on THRIFT-1058:
----------------------------------------
I stumbled across this because of the TFramedTransport. I wasn't using it for any of my services but then to support an HsHa server I started to wrap my normal socket transport in a framed one. Then it took a while but eventually I discovered that because of the framed transport the ByteBuffers I was getting had specific position and limit values set. So performing operations on the ByteBuffers caused several problems. Mostly that the buffers weren't reusable because I wasn't always reseting the position and limit. This inconsistent behavior could easily be solved by slicing the ByteBuffers and would make dealing them much easier.
> All ByteBuffers the client uses should be slice()ed
> ---------------------------------------------------
>
> Key: THRIFT-1058
> URL: https://issues.apache.org/jira/browse/THRIFT-1058
> Project: Thrift
> Issue Type: Bug
> Affects Versions: 0.6
> Reporter: ryan rawson
> Attachments: ByteBufferTest.java
>
>
> Right now the code does something like yay so:
> return ByteBuffer.wrap(array, pos, len);
> The only problem is that .wrap(byte[],int,int) returns a ByteBuffer with position equal to pos and limit set to pos+len. Which means users cant used 'rewind' or position(0).
> It would be cleaner if the contract was "ByteBuffers will have position=0, limit=how much data you have".
> The way to accomplish this would be to:
> return ByteBuffer.wrap(array, pos, len).slice();
> In exchange for 1 extra object alloc (which the GC can clean up quick) we get the benefit of a easier to use BB interface.
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (THRIFT-1058) All ByteBuffers the client uses
should be slice()ed
Posted by "Bryan Duxbury (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-1058?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13035651#comment-13035651 ]
Bryan Duxbury commented on THRIFT-1058:
---------------------------------------
What's the objection to slice()ing the buffers at use time? If it's already positioned at zero, then it won't cause any problems.
@Jeff - your own numbers show 20% overhead, which, while not the 100% I reported earlier, is nothing to ignore. (We've worked hard to get less than 20% in the past.)
> All ByteBuffers the client uses should be slice()ed
> ---------------------------------------------------
>
> Key: THRIFT-1058
> URL: https://issues.apache.org/jira/browse/THRIFT-1058
> Project: Thrift
> Issue Type: Bug
> Affects Versions: 0.6
> Reporter: ryan rawson
> Attachments: ByteBufferTest.java
>
>
> Right now the code does something like yay so:
> return ByteBuffer.wrap(array, pos, len);
> The only problem is that .wrap(byte[],int,int) returns a ByteBuffer with position equal to pos and limit set to pos+len. Which means users cant used 'rewind' or position(0).
> It would be cleaner if the contract was "ByteBuffers will have position=0, limit=how much data you have".
> The way to accomplish this would be to:
> return ByteBuffer.wrap(array, pos, len).slice();
> In exchange for 1 extra object alloc (which the GC can clean up quick) we get the benefit of a easier to use BB interface.
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira