Conversation
| Location, SchemaAsIpc, Ticket, | ||
| }; | ||
| use futures::{stream, Stream}; | ||
| use prost::Message; |
There was a problem hiding this comment.
Most of the changes in this file are actually clippy lints that weren't being picked up by CI (which I have also fixed)
| Ok(()) | ||
| } | ||
|
|
||
| fn prost_config() -> prost_build::Config { |
There was a problem hiding this comment.
Unfortunately prost_build::Config is not Clone so this was my workaround
alamb
left a comment
alamb
left a comment
There was a problem hiding this comment.
Looks good to me -- thank you @tustvold
I recommend we wait for a few days for more feedback on this API prior to merging
We could also perhaps add some documentation changes / explanation in the PR or code to help migration.
cc @avantgardnerio I suspect this will require some changes downstream in Ballista
|
|
||
| fn prost_config() -> prost_build::Config { | ||
| let mut config = prost_build::Config::new(); | ||
| config.bytes([".arrow"]); |
There was a problem hiding this comment.
Maybe we should put a note / link that this uses the Bytes API to avoid copies
tustvold
force-pushed
the
use-bytes-in-arrow-flight
branch
from
407ddb6 to
fddc134
Compare
|
I plan to merge this this afternoon unless there are any objections |
|
Benchmark runs are scheduled for baseline = f521e11 and contender = 8b84d4d. 8b84d4d is a master commit associated with this PR. Results will be available as each benchmark for each run completes. |
5 participants
Which issue does this PR close?
Closes #.
Rationale for this change
Prost supports zero-copy reading of binary payloads, internally slicing the original
Bytescomprising the HTTP frame read by hyper. This PR tweaks prost-build to generate structs withBytesinstead ofVec<u8>which should cut down on the amount of data copying during decode.TBC there will still be copies resulting from:
prost_types::AnyusesVecnotBytes(we could work around this in a subsequent PR)Bufferinvolves a memcpyWhat changes are included in this PR?
Are there any user-facing changes?