Skip to content

bpo-44661: Update property_descr_set to use vectorcall if possible. #27206

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Jump to bottom
Merged
merged 3 commits into from
Jul 19, 2021
Merged

bpo-44661: Update property_descr_set to use vectorcall if possible. #27206

merged 3 commits into from
Jul 19, 2021

Conversation

corona10
Copy link
Member

@corona10 corona10 commented Jul 17, 2021 •
edited
Loading

https://bugs.python.org/issue44661

Mean +- std dev: [property_base] 140 ns +- 5 ns -> [property_vectorcall] 125 ns +- 2 ns: 1.12x faster

@corona10 corona10 requested review from vstinner and pablogsal July 17, 2021 05:21
@corona10
Copy link
Member Author

cc @erlend-aasland

@corona10 corona10 changed the title bpo-44611: Update property_descr_set to use vectorcall if possible. bpo-44661: Update property_descr_set to use vectorcall if possible. Jul 17, 2021
erlend-aasland
erlend-aasland approved these changes Jul 17, 2021
View reviewed changes
@corona10
Copy link
Member Author

corona10 commented Jul 17, 2021 •
edited
Loading

@erlend-aasland FYI, I updated the benchmark with the result :) (old one was wrong actually it became more faster)

@Fidget-Spinner
Copy link
Member

Out of curiosity, is the benchmark from LTO+PGO build or non-debug build?

@corona10
Copy link
Member Author

corona10 commented Jul 17, 2021 •
edited
Loading

@Fidget-Spinner

Out of curiosity, is the benchmark from LTO+PGO build or non-debug build?

The attached result is tested on a non-debug build but the optimized build(Clang 12.0.0, macOS) also shows a similar result.

Mean +- std dev: [property_opt_base] 128 ns +- 5 ns -> [property_opt_vectorcall] 117 ns +- 2 ns: 1.09x faster

Fidget-Spinner
Fidget-Spinner approved these changes Jul 17, 2021
View reviewed changes
Copy link
Member

@Fidget-Spinner Fidget-Spinner left a comment

Choose a reason for hiding this comment

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

Wow those are some nice results for a few lines of code changed!

If I understand correctly, this won't slow down any other calls too right? The fast path with vectorcall should always be taken, since in most cases func should always be a python function/method which supports vectorcall.

@corona10
Copy link
Member Author

this won't slow down any other calls too right?

I believe that this code path is a good example to apply vectorcall :)

@corona10 corona10 merged commit 635bfe8 into python:main Jul 19, 2021
@corona10 corona10 deleted the bpo-44661 branch July 19, 2021 10:13
shihai1991 added a commit to shihai1991/cpython that referenced this pull request Jul 20, 2021
@shihai1991
Merge remote-tracking branch 'origin/main' into bpo_42035
56e8889 
* origin/main: (1146 commits)
  bpo-42064: Finalise establishing sqlite3 global state (pythonGH-27155)
  bpo-44678: Separate error message for discontinuous padding in binascii.a2b_base64 strict mode (pythonGH-27249)
  correct spelling (pythonGH-27076)
  bpo-44524: Add missed __name__ and __qualname__ to typing module objects (python#27237)
  bpo-27513: email.utils.getaddresses() now handles Header objects (python#13797)
  Clean up comma usage in Doc/library/functions.rst (python#27083)
  bpo-42238: Fix small rst issue in NEWS.d/. (python#27238)
  bpo-41972: Tweak fastsearch.h string search algorithms (pythonGH-27091)
  bpo-44340: Add support for building with clang full/thin lto (pythonGH-27231)
  bpo-44661: Update property_descr_set to use vectorcall if possible. (pythonGH-27206)
  bpo-44645: Check for interrupts on any potentially backwards edge (pythonGH-27216)
  bpo-41546: make pprint (like print) not write to stdout when it is None (pythonGH-26810)
  bpo-44554: refactor pdb targets (and internal tweaks) (pythonGH-26992)
  bpo-43086: Add handling for out-of-spec data in a2b_base64 (pythonGH-24402)
  bpo-44561: Update hyperlinks in Doc/distributing/index.rst (python#27032)
  bpo-42355: symtable.get_namespace() now checks whether there are multiple or any namespaces found (pythonGH-23278)
  bpo-44654: Do not export the union type related symbols (pythonGH-27223)
  bpo-44633: Fix parameter substitution of the union type with wrong types. (pythonGH-27218)
  bpo-44654: Refactor and clean up the union type implementation (pythonGH-27196)
  bpo-20291: Fix MSVC warnings in getargs.c (pythonGH-27211)
  ...
# for free to join this conversation on GitHub. Already have an account? # to comment
Reviewers

@erlend-aasland erlend-aasland erlend-aasland approved these changes

@Fidget-Spinner Fidget-Spinner Fidget-Spinner approved these changes

@vstinner vstinner Awaiting requested review from vstinner

@pablogsal pablogsal Awaiting requested review from pablogsal

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@corona10 @Fidget-Spinner @erlend-aasland @the-knights-who-say-ni @bedevere-bot