Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 34k
bpo-44525: Specialize simple Python calls.#29033
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
bpo-44525: Specialize simple Python calls. #29033
Uh oh!
There was an error while loading. Please reload this page.
Conversation
markshannon commented Oct 18, 2021 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
…onsume the argument references regardless of whether they succeed or fail.
| else{ | ||
| SPECIALIZATION_FAIL(CALL_FUNCTION, call_fail_kind(callable)); | ||
| fail = 1; | ||
| fail = -1; |
Fidget-SpinnerOct 20, 2021 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I set specialize_c_call to return 1 to indicate failure. You may have to change the return codes in specialize_c_call for consistency. Sorry!
markshannonOct 20, 2021 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should probably add an enum {SUCCESS, FAILURE} for clarity.
I wouldn't worry too much about it for now, we do much worse elsewhere.compile.c uses zero for success and non-zero for failure in some places, and the other way around in other places 😞
bedevere-bot commented Oct 20, 2021
🤖 New build scheduled with the buildbot fleet by @markshannon for commit 6f1bfab 🤖 If you want to schedule another build, you need to add the ":hammer: test-with-buildbots" label again. |
Fidget-Spinner commented Oct 20, 2021
It would be interesting to see the combined effects of Some extra food for thought: I wonder if we can remove the branch and inline python calling code from the generic |
| } | ||
| int deflen = cache1->defaults_len; | ||
| for (int i = 0; i < deflen; i++){ | ||
| PyObject *def = PyTuple_GET_ITEM(func->func_defaults, cache1->defaults_start+i); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if the compiler is smart enough to realize that func->func_defaults and cache1->defaults_start doesn't change between iterations? I'd say probably not. Perhaps we should micro-optimize and move this out of the loop?
markshannonOct 20, 2021 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The common case is that deflen == 0. In which case moving it out of loop could make it worse.
markshannon commented Oct 20, 2021
Fidget-Spinner commented Oct 20, 2021
Indeed, I was wondering what this PR + #26934 would yield, instead of just this PR alone. Nonetheless, this should pay for the specialization alone. |
markshannon commented Oct 20, 2021
|
Fidget-Spinner commented Oct 20, 2021
Great! Thank you for going through the trouble to benchmark. Other than the odd |
markshannon commented Oct 20, 2021
The usual buildbot failures. |
* main: (263 commits) bpo-45521: Fix a bug in the obmalloc radix tree code. (pythonGH-29051) bpo-45522: Allow to disable freelists on build time (pythonGH-29056) bpo-34451: Document prompt and output toggle feature in html tutorial (pythonGH-27105) bpo-44019: Add operator.call() to __all__ for the operator module (pythonGH-29110) bpo-45315: PyType_FromSpec: Copy spec->name and have the type own the memory for its name (pythonGH-29103) bpo-44220: Export PyStructSequence_UnnamedField in the limited API (pythonGH-26331) bpo-44174: [Enum] add reference to name mangling (pythonGH-29116) bpo-45548: add some missing entries to `Modules/Setup` (pythonGH-29115) bpo-35673: Add a public alias for namespace package __loader__ attribute (python#29049) bpo-45192: Fix a bug that infers the type of an os.PathLike[bytes] object as str (pythonGH-28323) bpo-45527: Don't count cache hits, just misses. (pythonGH-29092) bpo-45320: Remove long-deprecated inspect methods (pythonGH-28618) bpo-41374: Remove obsolete exclusion of netinet/tcp.h on Cygwin (pythonGH-21649) bpo-45532: Replace 'default' with 'main' as default in sys.version (pythonGH-29100) bpo-45464: [doc] Explain that subclassing multiple exceptions is fragile (pythonGH-29094) Cleanup a couple of comments left on PR 28775 post-merge. (pythonGH-29079) bpo-45536: Check OpenSSL APIs in configure (pythonGH-29088) Add PEPs 593 & 647 to list of PEPs at top of typing docs (pythonGH-29097) Add a comment about how to fix bogus test_host_resolution_bad_address failures (python#29085) bpo-44525: Specialize simple Python calls. (pythonGH-29033) ...
Complement to #26934
About 1% speedup
https://bugs.python.org/issue44525