[
https://issues.apache.org/jira/browse/THRIFT-6269?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Jens Geyer resolved THRIFT-6269.
--------------------------------
Fix Version/s: 0.25.0
Assignee: Jens Geyer
Resolution: Fixed
> Erlang: two avoidable per-byte costs in the socket transport and the JSON
> protocol
> ----------------------------------------------------------------------------------
>
> Key: THRIFT-6269
> URL: https://issues.apache.org/jira/browse/THRIFT-6269
> Project: Thrift
> Issue Type: Improvement
> Components: Erlang - Library
> Reporter: Jens Geyer
> Assignee: Jens Geyer
> Priority: Minor
> Fix For: 0.25.0
>
> Time Spent: 20m
> Remaining Estimate: 0h
>
> Two spots in {{lib/erl}} do per-byte work where the binding elsewhere works
> on binaries whole. Neither is a correctness problem; both are cheap to
> improve and they are filed together because they are one class.
> h2. 1. {{thrift_socket_transport.erl:71-82}} -- {{loop_recv}} reflattens the
> buffer on every pass
> {code:erlang}
> {ok, Data} ->
> Binary = iolist_to_binary([Buf, Data]),
> Give = min(iolist_size(Binary), ReadLen),
> loop_recv(State#t_socket{buffer = Binary}, ReadLen, ReadLen - Give)
> {code}
> {{iolist_to_binary/1}} copies everything received so far on each recursion,
> so a message arriving in n segments costs O(n^2) bytes copied. Carrying the
> iolist forward and flattening once, when the requested length is complete, is
> the same shape {{split_binary}} in the base clause already expects.
> h2. 2. {{thrift_json_protocol.erl:353-365}} -- {{read_all_1}} reads one byte
> per call
> {code:erlang}
> read_all_1(Transport0, IoList) ->
> {Transport1, Result} = thrift_transport:read(Transport0, 1),
> {code}
> One {{thrift_transport:read/2}} round trip per character of the message.
> Worth noting while looking at it: the {{{error, 'EOF'}}} clause below appears
> to be unreachable given what {{thrift_transport:read/2}} returns, which
> suggests the JSON-over-TCP server path has no test covering it.
> h2. Suggested
> Take them one at a time; each is self-contained. If the second is picked up,
> the apparently dead {{{error, 'EOF'}}} clause is worth resolving in the same
> pass -- either by covering that path with a test or by removing the clause.
> _Drafted with AI assistance (Claude Opus 5); reviewed and filed by Jens
> Geyer._
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
