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

Proposal: Performance optimization for Windows #136

Merged
merged 3 commits into from
Oct 23, 2021

Conversation

phprus
Copy link
Contributor

@phprus phprus commented Oct 3, 2021

  1. Apply clang-format
  2. Replace std::shared_ptr to std::unique_ptr because std::unique_ptr does not use reference counter.
  3. Replace std::shared_ptr<void> (for storing handle) to custom unique_handle class because std::shared_ptr use reference counter.

In all cases, a reference count is not needed, since the stored object exists in a single instance.

Replace std::shared_ptr<void> to std::unique_ptr is wrong, because std::unique_ptr doesn't call deleter on zero pointer and zero - is valid handle value.

@phprus
Copy link
Contributor Author

phprus commented Oct 20, 2021

@gulrak, I am sorry to distract you.
When you have time, please review this PR.

@gulrak gulrak added this to the v1.5.10 milestone Oct 21, 2021
@gulrak
Copy link
Owner

gulrak commented Oct 21, 2021

Thank you! I'll look into it this evening and as far as I can see it will be in 1.5.10. - I Needed to delay that release due to time constrains but really plan finishing it over the weekend, as I'm starting to now have a bit more time at my hands again.

@gulrak
Copy link
Owner

gulrak commented Oct 23, 2021

Nice, thank you! My only issue might be the different member marking style in the new helper class, but I can fix that when I wrap up the release.

@gulrak gulrak merged commit 09908b7 into gulrak:master Oct 23, 2021
@phprus phprus deleted the win-smart-ptr branch April 16, 2022 10:39
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.

2 participants