[
https://issues.apache.org/jira/browse/THRIFT-1035?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12980430#action_12980430
]
Bryan Duxbury commented on THRIFT-1035:
---------------------------------------
bq. don't you feel this exposes internal structures too much?
No, because if you expose any less, *you lose the performance benefit.*
bq. modifying one binary field (the underlying array) might modify another
binary field
This is impossible, if you're actually respecting the boundaries given to you
in the ByteBuffer.
bq. the re-use of underlying buffers (byte[]) is only done in selected
transports, for other transports (TSocket, THttpClient), the use of ByteBuffer
induces a performance penalty since not only do we have to allocate a new
byte[] but also a ByteBuffer that wraps that array.
Untrue. Firstly, users should never be using a raw TSocket - you always want a
BufferedTransport or a FramedTransport on top of it. The performance without it
is atrocious. Secondly, THttpClient *should* be using a Framed Transport or a
TMemoryInputTransport to wrap the underlying buffer/stream/whatever it gets
back for the same reasons. Thus, to my knowledge, every sane transport benefits
from the use of ByteBuffers. Even TDeserializer takes advantage of the
direct-buffer-access wrapping opportunities.
bq. But we're not talking about rewriting Thrift...
No, but we are talking about advancing it as a project. The switch to
ByteBuffers was a discussed means to improving performance and saving memory,
etc. As much as I like to make users happy, I don't think we should be bending
over backwards to make Thrift "easy" if it means we can't offer other things
like performance, flexibility, and consistency.
For that matter, I don't even buy the argument that it's that much easier. I
think it's just that it's the familiar way. There are helper methods to bridge
the gap, and the use of ByteBuffers encourages good application design.
> Container types containing binary data are parameterized with ByteBuffer in
> the generated Java code
> ---------------------------------------------------------------------------------------------------
>
> Key: THRIFT-1035
> URL: https://issues.apache.org/jira/browse/THRIFT-1035
> Project: Thrift
> Issue Type: Bug
> Components: Java - Compiler, Java - Library
> Affects Versions: 0.4, 0.5, 0.6, 0.7
> Environment: All
> Reporter: Mathias Herberts
> Attachments: THRIFT-1035-2.patch, THRIFT-1035.patch
>
>
> Since THRIFT-830, binary fields are internally handled using ByteBuffer.
> Release 0.4.0 was the first to expose the ByteBuffer to the outside world
> (replacing previous methods returning/accepting byte[]).
> THRIFT-882 lead to the methods accepting/returning byte[] being available
> again, as it was deemed more reasonable not to expose the ByteBuffer too much
> as their use could be cumbersome. This lead to 0.5.0 being backward
> compatible with 0.3.0 on the binary fields front.
> During that time, nobody noticed that container types that contained binary
> data had their generated Java code changed to collections parameterized with
> ByteBuffer instead of byte[].
> list<binary> -> List<ByteBuffer>
> set<binary> -> Set<ByteBuffer>
> map<binary,...> -> Map<ByteBuffer,...>
> map<...,binary> -> Map<...,ByteBuffer>
> This introduces confusion in the API and still exposes ByteBuffer when
> discussion on THRIFT-882 concluded this should be avoided.
> We need to provide a way to offer the original parameterization with byte[]
> as this will simplify working with that type of collection and thus will
> increase the odds of Thrift's adoption.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
