Skip to content

maint(perf): Optimize Numpy constructor to remove copies by value. #3183

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.

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Jump to bottom
Merged
Skylion007 merged 2 commits into from
Aug 9, 2021
Merged

maint(perf): Optimize Numpy constructor to remove copies by value. #3183

Skylion007 merged 2 commits into from
Aug 9, 2021

Conversation

@Skylion007
Copy link
Collaborator

@Skylion007 Skylion007 commented Aug 8, 2021

Description

  • Was looking through the numpy code and found a bunch of unneccessary copies. I added some std::move, but this constructor may want to use const-refs instead. I am not sure we have build coverage of this constructor hence why clang-tidy didn't detect this perf error.

Suggested changelog entry:

* Optimize numpy array construction with additional moves.

@Skylion007 Skylion007 requested review from henryiii and rwgk August 8, 2021 19:27
@Skylion007 Skylion007 merged commit ff590c1 into pybind:master Aug 9, 2021
@Skylion007 Skylion007 deleted the optimize-numpy-dtype-constructor branch August 9, 2021 16:48
@github-actions github-actions bot added the needs changelog Possibly needs a changelog entry label Aug 9, 2021
rwgk
rwgk reviewed Aug 9, 2021
View reviewed changes
Copy link
Collaborator

@rwgk rwgk left a comment

Choose a reason for hiding this comment

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

I'm late here, but that's great! :-)
Logging my approval only to track that I looked.

@henryiii henryiii removed the needs changelog Possibly needs a changelog entry label Sep 16, 2021
@rwgk rwgk mentioned this pull request Feb 10, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@rwgk rwgk rwgk left review comments

@henryiii henryiii henryiii approved these changes

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

@Skylion007 @rwgk @henryiii