Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

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

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Closed
dg-pb wants to merge2 commits intopython:mainfromdg-pb:remove_partial_fallback

Conversation

dg-pb
Copy link
Contributor

@dg-pbdg-pb commentedJun 20, 2024
edited
Loading

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 ofpartial being stuck intp_call after keywords are deleted even if function supportsvectorcall.
  2. Improves performance whenlen(pto-kw) != 0 and function supportsvectorcall.

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

Comparison of performance:

defsub(a,b):returna-bp=partial(sub,b=2)# Before%timeitp(1)# 240 ns# After%timeitp(1)# 180 ns# Using simpler but slower approach with `_PyObject_VectorcallDictTstate`%timeitp(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.

@rhettingerrhettinger requested review fromvstinner and removed request forrhettingerJune 20, 2024 23:39
@vstinner
Copy link
Member

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

@dg-pb
Copy link
ContributorAuthor

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

This one is a draft. Just an example of concept that works for currentfunctools.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 forpartial withPlaceholders:
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
Copy link
ContributorAuthor

Will issue a new PR forpartial with placeholders

@dg-pbdg-pb closed thisSep 26, 2024
@dg-pbdg-pb deleted the remove_partial_fallback branchSeptember 26, 2024 05:07
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@nineteendonineteendonineteendo left review comments

@vstinnervstinnerAwaiting requested review from vstinner

@rhettingerrhettingerAwaiting requested review from rhettingerrhettinger will be requested when the pull request is marked ready for reviewrhettinger is a code owner

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

Successfully merging this pull request may close these issues.

3 participants
@dg-pb@vstinner@nineteendo

[8]ページ先頭

©2009-2025 Movatter.jp