Use bytes in arrow-flight #3359
Conversation
| @@ -23,18 +23,14 @@ use arrow_flight::{ | |||
| 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)
| @@ -94,3 +94,9 @@ fn main() -> Result<(), Box<dyn std::error::Error>> { | |||
| // As the proto file is checked in, the build should not fail if the file is not found | |||
| 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
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. |
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?