Skip to content

Conversation

@skirpichev
Copy link
Member

@skirpichevskirpichev commented Dec 26, 2024

For reviewers: in the history included version, where PyLong_Export always set digits.

Benchmarkrefpatchno-value
Decimal(1<<7)735 nsnot significant721 ns: 1.02x faster
Decimal(1<<38)809 ns738 ns: 1.10x faster846 ns: 1.05x slower
Decimal(1<<300)1.93 us2.05 us: 1.06x slower1.99 us: 1.03x slower
Geometric mean(ref)1.01x faster1.01x slower

Benchmark hidden because not significant (1): Decimal(1<<3000)

benchmark code
# export_bench.pyimportpyperffromdecimalimportDecimalasDrunner=pyperf.Runner() i1, i2, i3, i4=1<<7, 1<<38, 1<<300, 1<<3000runner.bench_func('Decimal(1<<7)', D, i1) runner.bench_func('Decimal(1<<38)', D, i2) runner.bench_func('Decimal(1<<300)', D, i3) runner.bench_func('Decimal(1<<3000)', D, i4)

@skirpichev
Copy link
MemberAuthor

@vstinner, @serhiy-storchaka, please review.

I removed const's, added assert's, also handling value case now more simple. It seems, difference too small to measure on my system:

Benchmarkrefpatchonly-qset_i64
int(Decimal(1<<7))620 ns634 ns: 1.02x slower634 ns: 1.02x slower
int(Decimal(1<<38))697 ns728 ns: 1.04x slower719 ns: 1.03x slower
int(Decimal(1<<300))2.02 us2.04 us: 1.01x slower2.04 us: 1.01x slower
Geometric mean(ref)1.02x slower1.02x slower

Benchmark hidden because not significant (1): int(Decimal(1<<3000))

(*) only-qset_i64 - current version.

@serhiy-storchaka, if you prefer runtime checks over asserts - I'll cache layout parameters in the decimal module state (though, seems odd for me).

Copy link
Member

@serhiy-storchakaserhiy-storchaka left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks.

Copy link
Member

@vstinnervstinner left a comment

Choose a reason for hiding this comment

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

LGTM. I'm a little bit sad that the change makes the code a little bit slower, but I guess that's the price of abstraction.

@vstinnervstinner merged commit 879d287 into python:mainJan 6, 2025
43 checks passed
@vstinner
Copy link
Member

Merged, thanks.

@skirpichevskirpichev deleted the decimal-use-PyLong_Export/127937 branch January 6, 2025 10:54
srinivasreddy pushed a commit to srinivasreddy/cpython that referenced this pull request Jan 8, 2025
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@skirpichev@vstinner@serhiy-storchaka