pantShrey commented on PR #21882:
URL: https://github.com/apache/datafusion/pull/21882#issuecomment-4804303942

   @alamb I spent some time looking into the `spill_io` regression.
   
   From what I can tell, increasing the `ReaderStream` capacity somewhat 
restores the benchmark performance locally. I initially tried going up to **1 
MB**, but capacities around **256 KB** started failing the SQL logic tests, so 
for now I've pushed **128 KB**.
   
   My current understanding is that the previous `StreamReader` + `BufReader` 
implementation, despite having an 8 KB buffer, would typically read an entire 
IPC frame without yielding. With the current Tokio async stream, once the 
initial 
   8 KB is consumed, the task yields repeatedly while reading the remainder of 
the frame. Combined with the "copy" into the decoder's scratch buffer, this 
seems to add noticeable overhead for multi-MB frames/batches.
   
   ```rust
   tokio_util::io::ReaderStream::with_capacity(file, 128 * 1024)
   ```
   
   I've pushed the change mainly so the CI benchmarks can run and to get your 
thoughts. If this direction makes sense, or should I make it configurable 
instead?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to