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

bpo-44525: Specialize simple Python calls. #29033

Merged
merged 14 commits into from
Oct 20, 2021

Conversation

markshannon
Copy link
Member

@markshannon markshannon commented Oct 18, 2021

else {
SPECIALIZATION_FAIL(CALL_FUNCTION, call_fail_kind(callable));
fail = 1;
fail = -1;
Copy link
Member

@Fidget-Spinner Fidget-Spinner Oct 20, 2021

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!

Copy link
Member Author

@markshannon markshannon Oct 20, 2021

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 😞

@markshannon markshannon changed the title Specialize simple Python calls. bpo-44525: Specialize simple Python calls. Oct 20, 2021
@markshannon markshannon added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Oct 20, 2021
@bedevere-bot
Copy link

🤖 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.

@bedevere-bot bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Oct 20, 2021
@Fidget-Spinner
Copy link
Member

It would be interesting to see the combined effects of CALL_FUNCTION specialization altogether (ie from commit 3592980).

Some extra food for thought: I wonder if we can remove the branch and inline python calling code from the generic CALL_FUNCTION opcode? The specialization looks almost the same now and should cover all hot function cases.

}
int deflen = cache1->defaults_len;
for (int i = 0; i < deflen; i++) {
PyObject *def = PyTuple_GET_ITEM(func->func_defaults, cache1->defaults_start+i);
Copy link
Member

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?

Copy link
Member Author

@markshannon markshannon Oct 20, 2021

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
Copy link
Member Author

It would be interesting to see the combined effects of CALL_FUNCTION specialization altogether (ie from commit 3592980).

The benchmarks above compare this branch with main after #26934 was merged.

@Fidget-Spinner
Copy link
Member

It would be interesting to see the combined effects of CALL_FUNCTION specialization altogether (ie from commit 3592980).

The benchmarks above compare this branch with main after #26934 was merged.

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
Copy link
Member Author

It would be interesting to see the combined effects of CALL_FUNCTION specialization altogether (ie from commit 3592980).

The benchmarks above compare this branch with main after #26934 was merged.

Indeed, I was wondering what this PR + #26934 would yield, instead of just this PR alone. Nonetheless, this should pay for the specialization alone.

About 2% faster

@Fidget-Spinner
Copy link
Member

About 2% faster

Great! Thank you for going through the trouble to benchmark. Other than the odd unpack_sequence, the rest look good.

@markshannon
Copy link
Member Author

The usual buildbot failures.

@markshannon markshannon merged commit 8863a0f into python:main Oct 20, 2021
shihai1991 added a commit to shihai1991/cpython that referenced this pull request Oct 21, 2021
* 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)
  ...
@markshannon markshannon deleted the specialize-call-py-simple branch September 26, 2023 12:47
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.

4 participants