Skip to content

perf: use new PyLong* API for num-bigint feature#6144

Open
chirizxc wants to merge 27 commits into
PyO3:mainfrom
chirizxc:PyLongForBigInt
Open

perf: use new PyLong* API for num-bigint feature#6144
chirizxc wants to merge 27 commits into
PyO3:mainfrom
chirizxc:PyLongForBigInt

Conversation

@chirizxc

@chirizxc chirizxc commented Jun 20, 2026

Copy link
Copy Markdown
Contributor

Related: #6040

See codspeed comment for benchmark improvements.

@codspeed-hq

codspeed-hq Bot commented Jun 20, 2026

Copy link
Copy Markdown

Merging this PR will improve performance by 86.02%

⚡ 17 improved benchmarks
❌ 1 regressed benchmark
✅ 122 untouched benchmarks

Warning

Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

Benchmark BASE HEAD Efficiency
into_u128_u64_max 1.5 µs 2 µs -26.2%
into_bigint_huge_negative 23.4 µs 6.6 µs ×3.5
extract_biguint_negative_fail 7.3 µs 2.3 µs ×3.2
extract_biguint_zero 2,700.6 ns 916.9 ns ×2.9
into_bigint_huge_positive 17.3 µs 6.6 µs ×2.6
into_biguint_huge 17 µs 6.6 µs ×2.6
into_bigint_big_negative 5.2 µs 2.1 µs ×2.4
into_bigint_big_positive 4.4 µs 2.1 µs ×2.1
into_bigint_small 3.1 µs 1.5 µs ×2.1
into_biguint_big 4.3 µs 2.1 µs +98.96%
into_biguint_small 3.3 µs 2 µs +66.96%
extract_biguint_small 2.6 µs 1.6 µs +56.78%
extract_bigint_small 2.7 µs 1.7 µs +56.11%
extract_bigint_huge_negative 13 µs 8.5 µs +52.2%
into_biguint_zero 2.8 µs 2 µs +44.16%
extract_biguint_huge 11 µs 7.8 µs +41.24%
extract_bigint_huge_positive 11.7 µs 8.5 µs +37.62%
extract_bigint_big_negative 3.9 µs 3.4 µs +16.32%

Tip

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


Comparing chirizxc:PyLongForBigInt (9584a53) with main (b3aa7da)

Open in CodSpeed

@davidhewitt davidhewitt left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks, generally looks good on the from-python implementation; I think possible gains yet to be had in the to-python implementation.

Comment thread src/conversions/num_bigint.rs Outdated
Comment thread src/conversions/num_bigint.rs Outdated

#[cfg(all(not(Py_LIMITED_API), Py_3_14))]
{
if is_30bit_layout() {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Some observations on this path:

  • I would prefer to have this logic extracted into a generic function; I think it's easier to maintain than when it's deeply nested inside a macro.
  • I think we can avoid the intermediate Vec allocation from the digits and instead wrap the iterator coming out of the big-integer. pylong_from_digits needs an ExactSizeIterator, so it'll be a touch fiddly but I think still possible.
  • I think we lack benchmarks for the bigint to-python pathway, so possibly worth landing some of those before we think too hard about this.

@chirizxc chirizxc Jun 21, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

  • I think we lack benchmarks for the bigint to-python pathway, so possibly worth landing some of those before we think too hard about this.

#6148

@davidhewitt

Copy link
Copy Markdown
Member

Also, I think worth giving this a changed newsfragment to note to users we've optimized the bigint conversions on 3.14+.

@chirizxc chirizxc changed the title internal: try use new PyLong API for num-bigint feature perf: use new PyLong* API for num-bigint feature Jun 21, 2026
@chirizxc

Copy link
Copy Markdown
Contributor Author

⚠️ Different runtime environments detected

How exactly are GitHub machines assigned for running jobs, is it random?

Doesn't codspeed run benchmarks on the PR and main branches? Does it benchmark only the PR for every commit and compare the results to those saved from the main branch?

I mean, is there anything that can be done about the ⚠️ Different runtime environments detected error?

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.

2 participants