Skip to content

Conversation

@rwgk
Copy link
Collaborator

@rwgk rwgk commented May 26, 2022

Description

Contribution to / preparation for PR #3970. The inc_refs line in test_pytypes.py will need to be adjusted, which will effectively document the behavior change.

Full test coverage for all newly added functions, sensitive to the change from copy to move behavior.

I went through top-to-bottom in this copy of pytypes.h, undoing the changes under PR #3970 one-by-one, and verifying that the new test fails.

The newly introduced PYBIND11_HANDLE_REF_DEBUG might be useful for unit-testing other move-instead-of-copy optimizations. The main goal is to ensure that such optimizations do not accidentally get undone.

Suggested changelog entry:

@rwgk rwgk force-pushed the perf_attr-key-reference-stealing branch 2 times, most recently from 5dc2f31 to bc61683 Compare May 27, 2022 07:47
@rwgk rwgk changed the title simple performance measurements for PR #3970 unit test & simple performance measurements for PR #3970 May 27, 2022
@rwgk rwgk force-pushed the perf_attr-key-reference-stealing branch from e7d5c74 to 5fdfcdd Compare May 28, 2022 06:47
@pybind pybind deleted a comment from leslinice May 28, 2022
@pybind pybind deleted a comment from leslinice May 28, 2022
@rwgk rwgk changed the title unit test & simple performance measurements for PR #3970 addl unit tests for PR #3970 May 28, 2022
Copy link
Collaborator

@Skylion007 Skylion007 left a comment

Choose a reason for hiding this comment

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

@rwgk Looks good to me.

@rwgk
Copy link
Collaborator Author

rwgk commented May 31, 2022

@rwgk Looks good to me.

Great, thanks!
Unless you started on it already, I could strip out the PR #3970 changes from here and merge.
Then you could just rebase, adjust the expected inc_refs line, and merge your PR.

@Skylion007
Copy link
Collaborator

Skylion007 commented May 31, 2022

@rwgk Sounds good. Feel free to strip out the changes and merge.

@rwgk rwgk force-pushed the perf_attr-key-reference-stealing branch from 25c60cc to 0c63a12 Compare May 31, 2022 19:22
@rwgk rwgk marked this pull request as ready for review May 31, 2022 19:59
@rwgk rwgk merged commit 9f7b3f7 into pybind:master May 31, 2022
@github-actions github-actions bot added the needs changelog Possibly needs a changelog entry label May 31, 2022
@rwgk rwgk removed the needs changelog Possibly needs a changelog entry label May 31, 2022
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