Skip to content
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

Use try_for_each in into_py_dict #4647

Merged
merged 3 commits into from
Oct 26, 2024

Conversation

bschoenmaeckers
Copy link
Contributor

This applies the suggestion in #4493 (comment) to see if this will lead to any improved performance (or not 🤷‍♂️).

@davidhewitt
Copy link
Member

Thanks for following up with this! I think we actually lack benchmarks for this case (or at least I didn't spot any), so might need to add some? 🤔

@bschoenmaeckers
Copy link
Contributor Author

The dict_new benchmark uses into_py_dict but does not show any difference.

@bschoenmaeckers
Copy link
Contributor Author

I suspect it only has an advantage on the free-threaded build because of the critical-section wrapping the whole loop.

@davidhewitt davidhewitt added the CI-skip-changelog Skip checking changelog entry label Oct 26, 2024
Copy link
Member

@davidhewitt davidhewitt left a comment

Choose a reason for hiding this comment

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

Ah, yes.

I think it makes sense to proceed to merge this; as I understand it, for complex iterators this is better for the optimizer and it's definitely better for our own free-threaded iterators!

@davidhewitt davidhewitt added this pull request to the merge queue Oct 26, 2024
Merged via the queue into PyO3:main with commit fbeb2b6 Oct 26, 2024
43 of 45 checks passed
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
CI-skip-changelog Skip checking changelog entry
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants