Skip to content

Set default read_size to 1 for backwards compatibility#275

Merged
agronholm merged 6 commits intoagronholm:masterfrom
andreer:default-read-size-1
Feb 28, 2026
Merged

Set default read_size to 1 for backwards compatibility#275
agronholm merged 6 commits intoagronholm:masterfrom
andreer:default-read-size-1

Conversation

@andreer
Copy link
Copy Markdown
Contributor

@andreer andreer commented Jan 19, 2026

Changes

Set default read_size to 1.

Fixes #272

The buffered reads introduced in 5.8.0 could cause issues when code needs to access the stream position after decoding. This changes the default back to 1 (matching 5.7.1 behavior) while still allowing users to opt-in to faster decoding by passing read_size=4096.

Implementation details:

  • Use function pointer dispatch to eliminate runtime checks for read_size=1
  • Skip buffer allocation entirely for unbuffered path
  • Add read_size parameter to load() and loads() for API completeness

Checklist

If this is a user-facing code change, like a bugfix or a new feature, please ensure that
you've fulfilled the following conditions (where applicable):

  • You've added tests (in tests/) which would fail without your patch
  • You've updated the documentation (in docs/), in case of behavior changes or new
    features
  • You've added a new changelog entry (in docs/versionhistory.rst).

The buffered reads introduced in 5.8.0 could cause issues when code needs
to access the stream position after decoding. This changes the default
back to 1 (matching 5.7.1 behavior) while allowing users to opt-in to
faster decoding by passing read_size=4096.

Implementation details:
- Use function pointer dispatch to eliminate runtime checks for read_size=1
- Skip buffer allocation entirely for unbuffered path
- Add read_size parameter to load() and loads() for API completeness
@agronholm
Copy link
Copy Markdown
Owner

Note: when you mark those check boxes, don't add extra spaces around the X as it prevents them from being rendered as checkboxes on the github website.

@coveralls
Copy link
Copy Markdown

coveralls commented Jan 19, 2026

Coverage Status

coverage: 94.58%. remained the same
when pulling 6737e04 on andreer:default-read-size-1
into 3fb7c5b on agronholm:master.

Comment on lines +50 to +52
// Forward declarations for read dispatch functions
static int fp_read_unbuffered(CBORDecoderObject *, char *, Py_ssize_t);
static int fp_read_buffered(CBORDecoderObject *, char *, Py_ssize_t);
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Do you suppose the run-time check is costly enough to warrant this?

Copy link
Copy Markdown
Owner

@agronholm agronholm left a comment

Choose a reason for hiding this comment

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

I think the read-ahead warrants a mention in the docs, as the optimization now requires an opt-in.

@agronholm
Copy link
Copy Markdown
Owner

@andreer Are you up to the task of adding the appropriate documentation paragraph? What are your intentions?

@agronholm
Copy link
Copy Markdown
Owner

@andreer ping - do you intend to finish this PR, or would you like me to take over?

@agronholm
Copy link
Copy Markdown
Owner

For anyone watching, I think I'll finish the code and docs myself in a few days.

Copy link
Copy Markdown
Owner

@agronholm agronholm left a comment

Choose a reason for hiding this comment

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

I've added a docs section to explain how to tune the performance.

@agronholm agronholm merged commit e61a5f3 into agronholm:master Feb 28, 2026
14 checks passed
@agronholm
Copy link
Copy Markdown
Owner

Thanks!

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.

Sequential decoding does not behave as expected

3 participants