Skip to content
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

Optimize to_binary() function and to_bytes() method #539

Merged
merged 1 commit into from
Dec 26, 2024

Conversation

skirpichev
Copy link
Contributor

@skirpichev skirpichev commented Dec 23, 2024

It seems, that using mpn_get_str() is more efficient than generic mpz_export().

Some benchmarks are here:
#404 (comment)

Not sure what else we can do for #404. In the python-gmp I've added also the __reduce__ dunded method. This seems slightly better than rely on copyreg to support pickling:

Benchmark ref patch gmp
dumps(1<<7) 23.9 us 23.8 us: 1.01x faster 22.6 us: 1.06x faster
dumps(1<<38) 24.0 us 23.9 us: 1.01x faster 22.7 us: 1.06x faster
dumps(1<<300) 24.1 us 23.8 us: 1.01x faster 22.9 us: 1.05x faster
dumps(1<<3000) 26.8 us 25.2 us: 1.07x faster 23.8 us: 1.13x faster
Geometric mean (ref) 1.02x faster 1.07x faster

Can we add pickling to the gmpy2 with even less overhead? I don't know.

But if we avoid pickle machinery, you can see noticeable performance boost for small numbers too:

Benchmark to_binary-ref to_binary-patch
dumps(1<<7) 323 ns 300 ns: 1.08x faster
dumps(1<<38) 352 ns 315 ns: 1.12x faster
dumps(1<<300) 603 ns 436 ns: 1.39x faster
dumps(1<<3000) 3.17 us 1.57 us: 2.02x faster
Geometric mean (ref) 1.35x faster

New code seems faster than int.to_bytes() roughly from 500bit numbers on my system.

benchmark code
# pickle-bench.py
import pyperf
from pickle import dumps
from gmpy2 import mpz


runner = pyperf.Runner()
ints = "1<<7", "1<<38", "1<<300", "1<<3000"

for v in ints:
    n = mpz(eval(v))
    v = "dumps(" + v + ")"
    runner.bench_func(v, dumps, n)
# pickle-bench2.py
import pyperf
from gmpy2 import mpz, to_binary


runner = pyperf.Runner()
ints = "1<<7", "1<<38", "1<<300", "1<<3000"

for v in ints:
    n = mpz(eval(v))
    v = "dumps(" + v + ")"
    runner.bench_func(v, to_binary, n)

It seems, that using mpn_get_str() is more efficient than generic
mpz_export().

Some benchmarks are here:
aleaxit#404 (comment)

Not sure what else we can do for aleaxit#404.  In the python-gmp I've added
also the `__reduce__` dunded method.  This seems slightly better than
rely on copyreg to support pickling:

| Benchmark      | ref     | patch                 | gmp                   |
|----------------|:-------:|:---------------------:|:---------------------:|
| dumps(1<<7)    | 23.9 us | 23.8 us: 1.01x faster | 22.6 us: 1.06x faster |
| dumps(1<<38)   | 24.0 us | 23.9 us: 1.01x faster | 22.7 us: 1.06x faster |
| dumps(1<<300)  | 24.1 us | 23.8 us: 1.01x faster | 22.9 us: 1.05x faster |
| dumps(1<<3000) | 26.8 us | 25.2 us: 1.07x faster | 23.8 us: 1.13x faster |
| Geometric mean | (ref)   | 1.02x faster          | 1.07x faster          |

Can we add pickling to the gmpy2 with even less overhead?  I don't know.

But if we avoid pickle machinery, you can see noticeable performance
boost for small numbers too:

| Benchmark      | to_binary-ref | to_binary-patch       |
|----------------|:-------------:|:---------------------:|
| dumps(1<<7)    | 323 ns        | 300 ns: 1.08x faster  |
| dumps(1<<38)   | 352 ns        | 315 ns: 1.12x faster  |
| dumps(1<<300)  | 603 ns        | 436 ns: 1.39x faster  |
| dumps(1<<3000) | 3.17 us       | 1.57 us: 2.02x faster |
| Geometric mean | (ref)         | 1.35x faster          |

New code seems faster than int.to_bytes() roughly from 500bit numbers on
my system.
@casevh casevh merged commit dc08582 into aleaxit:master Dec 26, 2024
11 checks passed
@skirpichev skirpichev deleted the use-mpn_get_str/404 branch December 26, 2024 01:47
@skirpichev
Copy link
Contributor Author

N.B. this patch has two helpers, added to src/gmpy2_macros.h. I'm not sure if it's a right place or they correctly named.

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