FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

gh-119109: functool.partial vectorcall supports pto->kw & fallback to tp_call removed by dg-pb · Pull Request #120783 · python/cpython · GitHub

/ cpython Public

gh-119109: functool.partial vectorcall supports pto->kw & fallback to tp_call removed - #120783

Closed
dg-pb wants to merge 2 commits into
python:mainfrom
dg-pb:remove_partial_fallback
Closed

gh-119109: functool.partial vectorcall supports pto->kw & fallback to tp_call removed#120783
dg-pb wants to merge 2 commits into
python:mainfrom
dg-pb:remove_partial_fallback

Conversation

dg-pb commented Jun 20, 2024
edited
Loading

Copy link
Copy Markdown
Contributor

Although comment stated that "merging keyword arguments for vector call is messy" (not exact quote), but I have found that it is fairly straight forward.

This achieves 2 goals:

  1. Solves the raised issue of partial being stuck in tp_call after keywords are deleted even if function supports vectorcall.
  2. Improves performance when len(pto-kw) != 0 and function supports vectorcall.

Number 1. is solved better than it was in #119125 as there are no more switching between methods and faster vectorcall is used whenever appropriate. Also, this does not require additional variable in class struct.

Comparison of performance:

def sub(a, b):
    return a - b

p = partial(sub, b=2)
# Before
%timeit p(1)    # 240 ns
# After
%timeit p(1)    # 180 ns
# Using simpler but slower approach with `_PyObject_VectorcallDictTstate`
%timeit p(1)    # 200 ns

This is only initial attempt and I think the code can be made simpler. (much simpler if decide to use _PyObject_VectorcallDictTstate).

This will need to be adjusted to #119827 so do not merge.

Comment thread Modules/_functoolsmodule.c Outdated
Comment thread Modules/_functoolsmodule.c Outdated
rhettinger requested review from vstinner and removed request for rhettinger June 20, 2024 23:39

Copy link
Copy Markdown
Member

Is the PR a draft or is it ready for review?

dg-pb commented Jun 24, 2024

Copy link
Copy Markdown
Contributor Author

Is the PR a draft or is it ready for review?

This one is a draft. Just an example of concept that works for current functools.partial.

Reason being that #119827 will need a different implementation, which I have, but still waiting for reply how to go about it (#119827 (comment)):
a) Make it part of #119827
b) Wait after it is merged and issue a new PR

Here is implementation that works for partial with Placeholders:
https://gist.github.com/dg-pb/45f6d3af262056aaea1acd4ce29ee4dd

It can be reviewed - sooner or later it will become PR. It is contained within one function - no documentation updates or anything additional is needed.

Let me know if there is a good way to issue a separate PR on top of the one which is not yet merged - I am not aware of such.

dg-pb commented Sep 26, 2024

Copy link
Copy Markdown
Contributor Author

Will issue a new PR for partial with placeholders

dg-pb closed this Sep 26, 2024
dg-pb deleted the remove_partial_fallback branch September 26, 2024 05:07
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
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.

3 participants


Back | FazBrowse Home | New Git URL