Skip to content

gh-144650: support Vectorcall for _decimal.Decimal's - #158207

Open
skirpichev wants to merge 13 commits into
python:mainfrom
skirpichev:decimal-ac/144650
Open

skirpichev wants to merge 13 commits into
python:mainfrom
skirpichev:decimal-ac/144650

Conversation

@skirpichev

@skirpichev skirpichev commented Sep 26, 2026 •

Copy link
Copy Markdown
Member

This shows almost no effect on "official" pyperformance benchmarks (see python/pyperformance#473):

decimal_pi: Mean +- std dev: [ref] 1.92 sec +- 0.02 sec -> [patch] 1.89 sec +- 0.02 sec: 1.02x faster

Benchmark hidden because not significant (2): decimal_factorial, telco

Geometric mean: 1.01x faster

But on micro-benchmarks it show some speedup:

Decimal(0): Mean +- std dev: [ref-micro] 591 ns +- 10 ns -> [patch-micro] 513 ns +- 12 ns: 1.15x faster
Decimal(123): Mean +- std dev: [ref-micro] 596 ns +- 22 ns -> [patch-micro] 515 ns +- 26 ns: 1.16x faster
Decimal('123'): Mean +- std dev: [ref-micro] 764 ns +- 16 ns -> [patch-micro] 677 ns +- 24 ns: 1.13x faster

Geometric mean: 1.15x faster

(Default ./configure options, gcc 14.2)

Details
# bench.py
import pyperf
from decimal import Decimal

runner = pyperf.Runner()
for x in [0, 123, '123']:
    h = f"Decimal({x!r})"
    runner.bench_func(h, Decimal, x)

@skirpichev skirpichev changed the title Support Vectorcall for _decimal.Decimal's gh-144650: support Vectorcall for _decimal.Decimal's Sep 26, 2026
@cmaloney

Copy link
Copy Markdown
Contributor

#157968 may help with the build failures (changes how the assert works which should make it better across module boundaries)

@skirpichev

Copy link
Copy Markdown
Member Author

Thanks. The Decimal is only possible candidate being a heap type in the stdlib. That was worked before in skirpichev#18

@skirpichev
skirpichev marked this pull request as draft September 26, 2026 03:02
@skirpichev skirpichev self-assigned this Sep 26, 2026
@cmaloney

Copy link
Copy Markdown
Contributor

👍 hopefully will get more of these landed soon :). str and list still need some special work for stable performance over the change which I'm still root causing (likely compiler inlining vs. existing hand inlining). If there are ways I can help enable / particular edge cases you find let me know. Otherwise planning to make PRs as I have time + performance results for existing cases across the codebase (int, float, str, list, bytes, bytearray, bool, filter, set, frozenset are on my radar).

cmaloney and others added 3 commits September 26, 2026 06:31
Convert the static types to heap types. Move empty init vectorcall to
share with existing no-arg __init__ case. Add support for no-argument
vectorcall so the no-arg __init__ cases actually work (rather than
adding an arg). Produce the same error string as the PyArg helpers do.

The vectorcall types no longer have a C address for the generated
`Py_Is` so instead compare the slot function instead. This drops the
requirement that a class using vectorcall declare a type object.

Co-Authored-By: Claude Fable 5.1 <[email protected]>

(cherry-picked from python#157968)
@skirpichev
skirpichev marked this pull request as ready for review September 26, 2026 03:41
@skirpichev skirpichev removed their assignment Oct 2, 2026
@bedevere-app bedevere-app Bot added the type-feature A feature request or enhancement label Oct 6, 2026
@skirpichev

skirpichev commented Oct 8, 2026 •

Copy link
Copy Markdown
Member Author

CC @vstinner, does it make sense for you?

Edit: N.B., this depends on #157968.

# Conflicts:
#	Modules/clinic/_testclinic.c.h
@skirpichev skirpichev closed this Oct 17, 2026
@skirpichev
skirpichev deleted the decimal-ac/144650 branch October 17, 2026 03:07
@vstinner

Copy link
Copy Markdown
Member

Did you close it because you consider that it's not worth it?

@skirpichev

Copy link
Copy Markdown
Member Author

I think it still worth it, assuming micro-benchmarks. It's for free, anyway.

But it's on top of #157968. If you are ok, I'll reopen this one that pr (or some its alternative) will be merged.

@skirpichev skirpichev reopened this Oct 23, 2026

@vstinner vstinner 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.

Benchmark results were posted 1 month ago, but the PR changed in the meanwhile. Would it be possible to re-run benchmarks on the latest PR version?

an exact conversion. If the result does not meet the restrictions
for an mpd_t, fail with InvalidOperation. */
static PyObject *
static inline PyObject *

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.

Is it useful to mark the function as "inline"? Does it impact performance?

@vstinner

Copy link
Copy Markdown
Member

See also #73487 which converted many Decimal methods to vectorcall using Argument Clinic.

@skirpichev

Copy link
Copy Markdown
Member Author

#158207 (comment) were posted 1 month ago, but the PR changed in the meanwhile. Would it be possible to re-run benchmarks on the latest PR version?

In fact, you can see that benchmarks already updated. Not so much, as before. #156034 shows better numbers.

Now included telco benchmark.

Is it useful to mark the function as "inline"? Does it impact performance?

This is something that was suggested in #160115.

You can compare my previous benchmark run. IMO, the difference looks more or less as a noise:
#160115 (comment)

Not sure if this worth reversion. That function used only once, anyway.

@cmaloney

cmaloney commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor

I don't mind the inline keyword, it's a compiler hint that is largely ignored (which is why forceinline exists..). Compilers decided people perf tested and added then never maintained ><. In this case PyDecType_FromObjectExact inlines into the _new_impl with or without it in the experimentation I've done. It doesn't inline into the vectorcall handler because there are two callers of _new_impl and the compiler decides not to proceed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

awaiting review type-feature A feature request or enhancement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants