Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor/shrink modernize scopedtimer (Alternative to #13236) #13258

Merged

Conversation

Swiftb0y
Copy link
Member

#13236 but squashed and on top of main

src/util/timer.h Outdated
#endif
m_maybeTimer = std::make_optional<Timer>(std::move(strKey), compute);
//
auto assembledKey = QString(std::forward<T>(key));
Copy link
Member

Choose a reason for hiding this comment

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

This one is unfortunately not for free, because the compiler decides to create a local String copy:
https://godbolt.org/z/67M4MTc6j
Here is the `const QString&``version:
https://godbolt.org/z/9WbMedoej

Copy link
Member Author

Choose a reason for hiding this comment

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

But in both cases, it still holds that capacity() == 0, thus the copy is not heap-allocated, so I don't really see a problem. Anyways, wdyt of the fixup. It avoids the construction of the temporary by use of an IILE.

@Swiftb0y Swiftb0y requested a review from daschuer May 28, 2024 08:06
@Swiftb0y Swiftb0y force-pushed the refactor/shrink-modernize-scopedtimer-main branch 3 times, most recently from 3636289 to 46c7faa Compare May 30, 2024 10:19
@Swiftb0y Swiftb0y force-pushed the refactor/shrink-modernize-scopedtimer-main branch from 46c7faa to 4ece08b Compare May 30, 2024 10:20
@Swiftb0y
Copy link
Member Author

Ready for review @daschuer. Thank you.

Copy link
Member

@daschuer daschuer left a comment

Choose a reason for hiding this comment

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

LGTM, Thank you.

@daschuer daschuer merged commit c0407e5 into mixxxdj:main May 30, 2024
13 checks passed
@Swiftb0y
Copy link
Member Author

Thank you.

@Swiftb0y Swiftb0y deleted the refactor/shrink-modernize-scopedtimer-main branch May 31, 2024 07:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants