Skip to content

Implement BufRead for accounts_db BufferedReader #6827

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Jul 8, 2025

Conversation

kskalski
Copy link

@kskalski kskalski commented Jul 3, 2025

Problem

BufferedReader uses a custom API to implement the same access pattern as BufRead trait (though it also relaxes some of its constraints and adds a couple of extensions).
The custom API makes it harder to understand and reason about, but also it's harder to implement/substitute with an alternative implementation (e.g. io_uring read-ahead buffered reader).

Summary of Changes

BufferedReader provides all the functionality of BufRead, so the custom API is converted to use of fill_buf and consume. The extra functionality like access to current offset and setting min required data is kept as pub API of BufferedReader (when alternative impl is available, those should be elevated to separete trait that will extend BufRead).

@codecov-commenter
Copy link

codecov-commenter commented Jul 3, 2025

Codecov Report

Attention: Patch coverage is 89.83051% with 12 lines in your changes missing coverage. Please review.

Please upload report for BASE (master@0de8717). Learn more about missing BASE report.
Report is 2 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff            @@
##             master    #6827   +/-   ##
=========================================
  Coverage          ?    83.3%           
=========================================
  Files             ?      853           
  Lines             ?   378550           
  Branches          ?        0           
=========================================
  Hits              ?   315498           
  Misses            ?    63052           
  Partials          ?        0           
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@kskalski kskalski force-pushed the ks/buf_read branch 2 times, most recently from a7969a8 to 7e680ea Compare July 7, 2025 15:09
@kskalski kskalski marked this pull request as ready for review July 7, 2025 15:22
Copy link

@cpubot cpubot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great improvement, thank you!

Copy link

@brooksprumo brooksprumo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:shipit:

@kskalski kskalski merged commit f39ea99 into anza-xyz:master Jul 8, 2025
41 checks passed
@kskalski kskalski deleted the ks/buf_read branch July 8, 2025 07:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants