grpc-go icon indicating copy to clipboard operation
grpc-go copied to clipboard

mem: allow using io.WriterTo with a io.LimitedReader

Open GiedriusS opened this issue 2 months ago • 4 comments

The caller of this function can wrap the io.Reader in a io.LimitedReader. This happens if some max message size is set. If so, this io.WriterTo check doesn't work anymore. Work around this by checking if it is maybe a io.LimitedReader.

Overall, the problem I'm trying to solve is that the constant

		buf := pool.Get(readAllBufSize)

32KiB is way too much in our use case. Messages are typically at max about only 1KiB in size so we always overallocate by ~31KiB in the best case scenario so we want to use the io.WriterTo branch so that we could appropriately size the buffer.

Is this OK? Any suggestions on how to make the code prettier? Also, maybe some suggestions on how to do something like io.LimitedReader on the io.WriterTo?

GiedriusS avatar Nov 06 '25 11:11 GiedriusS

Codecov Report

:white_check_mark: All modified and coverable lines are covered by tests. :warning: Please upload report for BASE (master@7472d57). Learn more about missing BASE report. :warning: Report is 15 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff            @@
##             master    #8697   +/-   ##
=========================================
  Coverage          ?   83.25%           
=========================================
  Files             ?      416           
  Lines             ?    32306           
  Branches          ?        0           
=========================================
  Hits              ?    26895           
  Misses            ?     4027           
  Partials          ?     1384           
Files with missing lines Coverage Δ
mem/buffer_slice.go 96.57% <100.00%> (ø)
:rocket: New features to boost your workflow:
  • :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

codecov[bot] avatar Nov 06 '25 11:11 codecov[bot]

@ash2k any thoughts?

GiedriusS avatar Nov 10 '25 13:11 GiedriusS

@arjan-bal : Can you please take a first look and see if this seems ok.

easwars avatar Nov 11 '25 03:11 easwars

This PR is labeled as requiring an update from the reporter, and no update has been received after 6 days. If no update is provided in the next 7 days, this issue will be automatically closed.

github-actions[bot] avatar Nov 26 '25 08:11 github-actions[bot]