Skip to content

strparse: speed up number parsing - #22993

Closed
bagder wants to merge 1 commit into
masterfrom
bagder/faster-numparser
Closed

bagder wants to merge 1 commit into
masterfrom
bagder/faster-numparser

Conversation

@bagder

@bagder bagder commented Sep 18, 2026

Copy link
Copy Markdown
Member

By using a cutoff value for looping simpler while not close to the max.

Runs the numparser test 15-17% faster for me.

By using a cutoff value for looping simpler while not close to the max.

Runs the numparser test 15-17% faster for me.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟢 Approval recommended

The cutoff calculation preserves overflow behavior and existing boundary coverage is adequate.

Pull request overview

Optimizes numeric parsing by avoiding repeated overflow division checks when safely below the maximum.

Changes:

  • Adds a cutoff-based fast path while preserving boundary overflow checks.
File summaries
File Description
lib/curlx/strparse.c Optimizes decimal, hexadecimal, and octal parsing.
Review details
  • Files reviewed: 1/1 changed files
  • Comments generated: 0
  • Review effort level: Balanced

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@bagder
bagder marked this pull request as ready for review September 18, 2026 12:43
@testclutch

Copy link
Copy Markdown

Analysis of PR #22993 at 45f44fc9:

Test ../../tests/http/test_02_download.py::TestDownload::test_02_34_max_total_conns[True-0-h2] failed, but it has been 0.4% flaky lately, so it's probably NOT a fault of the PR. Note that this test has failed in 2 different CI jobs (the link just goes to one of them). Note that this CI job has had a number of other flaky tests recently (2, to be specific) so it may be that this failure is rather a systemic issue with this job and not with this specific PR.

Test 3232 failed, but it has been 77.0% flaky lately, so it's probably NOT a fault of the PR. Note that this test has failed in 2 different CI jobs (the link just goes to one of them). Note that this CI job has had a number of other flaky tests recently (4, to be specific) so it may be that this failure is rather a systemic issue with this job and not with this specific PR.

Test 3231 failed, but it has been 86.9% flaky lately, so it's probably NOT a fault of the PR. Note that this test has failed in 2 different CI jobs (the link just goes to one of them). Note that this CI job has had a number of other flaky tests recently (4, to be specific) so it may be that this failure is rather a systemic issue with this job and not with this specific PR.

Generated by Testclutch

@bagder

bagder commented Sep 18, 2026

Copy link
Copy Markdown
Member Author

The numparser perf chart: https://curl.se/perf/index.html#numparser

@bagder bagder closed this in 15bc69a Sep 18, 2026
@bagder
bagder deleted the bagder/faster-numparser branch September 18, 2026 20:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants