Don't count negative sign towards the integer digit limit

[lolopinto]

Negative JSON integers with exactly 4300 digits were rejected because the leading minus sign was incorrectly counted towards the integer digit limit.

This changes it so that we count the digits independent of the sign and adds tests.

jiter.from_json(b"-" + b"9" * 4300)

This now parses successfully.

[codecov]

Codecov Report

❌ Patch coverage is 87.50000% with 1 line in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
crates/jiter/src/number_decoder.rs	87.50%	0 Missing and 1 partial ⚠️

📢 Thoughts on this report? Let us know!

[codspeed-hq]

Merging this PR will regress 1 benchmark

⚡ 2 improved benchmarks
❌ 1 regressed benchmark
✅ 58 untouched benchmarks
⏩ 71 skipped benchmarks¹

Warning

Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

	Mode	Benchmark	BASE	HEAD	Efficiency
❌	WallTime	pass2_jiter_value	5.6 µs	6 µs	-6.78%
⚡	WallTime	string_array_jiter_value	5.3 µs	4.9 µs	+8.36%
⚡	WallTime	python_parse_true_array	3.9 µs	3.7 µs	+5.27%

Tip

Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.

---

_{Comparing lolopinto:negative-integer-boundary (9b7f894) with main (1de3b88)}

Footnotes

1. 71 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

[lolopinto]

@codspeedbot fix this regression

[lolopinto]

I pointed an agent at this and it made some changes and it thinks the rest is because we don't have an apples to apples comparison

[samuelcolvin]

looks like this damaged performance.

[lolopinto]

Looks like the regression is down to this:

• the base used num-bigint 0.4.6
• the PR resolved 0.4.8, which introduced a substantially slower allocation path for large integers.

The benchmark only contains positive integers, and testing with identical dependencies shows no meaningful performance regression from the PR itself.

Can we redo a base? or pin to 0.4.6?

[davidhewitt]

@codspeedbot explain this regression

[codspeed-hq]

@codspeedbot explain this regression

Your diagnosis is right — I reproduced it locally. The reported regression is not caused by this PR; it comes from two differences between the base run and the head run.

What I measured

I ran crates/jiter/benches/main.rs under the simulation instrument locally (callgrind, same machine, same toolchain) in three configurations. Numbers are instruction counts (Ir).

1. Base (785cf75) vs head (edc3e7b), with an identical lockfile (num-bigint 0.4.8) — i.e. the PR diff on its own:

Benchmark	Δ Ir
massive_ints_array_jiter_iter	+0.69%
massive_ints_array_jiter_value	+0.30%
big_jiter_value	-0.13%
bigints_array_jiter_value	-0.03%
short_numbers_jiter_value	+0.03%

The only non-trivial cost of the diff is on the skip path, which now does one extra comparison per integer chunk in NumberRange::decode: massive_ints_array_jiter_skip +2.7%, short_numbers_jiter_skip +2.3%, big_jiter_skip +1.6%. None of those were flagged in the report.

2. Same head commit, only num-bigint changed (0.4.6 → 0.4.8):

Benchmark	Δ Ir
massive_ints_array_jiter_iter	+22.2%
massive_ints_array_jiter_value	+15.6%
short_numbers_jiter_value	+15.0%
bigints_array_jiter_value	+9.6%
big_jiter_value	+9.6%
floats_array_jiter_value	+6.3%

That is the same set of benchmarks, in the same order of magnitude, as the CodSpeed report (-18.06% / -12.54% / -12.37% / -9.86% / -7.45% / -5.23%). *_jiter_skip and *_serde_value are untouched by the dependency change, which is consistent with the report too. JsonValue is 32 bytes under both versions, so this is the bigint parse/allocation path, not a layout change.

3. A second, independent confounder: the two runs were not on the same hardware. The base run (main 785cf75, 29 Jun) ran on an Intel Xeon Platinum 8370C, the head run (16 Jul) on an AMD EPYC 9V74. Every benchmark in the comparison crossed that boundary — that's the "Different runtime environments" banner on the report. Simulation derives its cache model from the physical CPU, so this moves individual benchmarks by several percent on identical code.

What to do

No need to pin 0.4.6. Since #265, Cargo.lock is committed on main and pins num-bigint 0.4.8, so the dependency drift that caused this can't happen again — the branch just predates that commit. Merging (or rebasing onto) current main is enough to get an apples-to-apples run; main has also since moved to CodSpeed's dedicated runners, which removes the runner-CPU variability as well.

I checked that git merge origin/main into this branch is clean (only number_decoder.rs auto-merges, and the PR's logic survives intact) and the test suite passes on the merged tree. I haven't pushed it: this PR's head is on your fork, and I can only push to pydantic/jiter, so the merge needs to come from your side.

Summary

• Warming up the performance wizard...
• Handling comment — Comment handling finished successfully

_{Learnings updated. See what CodSpeed remembers about this repository.}

_{Was this response helpful? React with 👍 or 👎 to help us improve CodSpeedBot.}