-
Notifications
You must be signed in to change notification settings - Fork 290
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
Add zero-copy make_mut #695
Conversation
Why is |
I mean I guess we could but we already have to handle the copy case in each to_mut vtable function so I dont really see the point in not using that. Plus it there is a small chance that a concurrent thread released the shared since we use acquire vs relaxed ordering. |
Sorry I didnt have to work on it more, I will try this week. @braddunbar if you have for a review I would appreciate 🙏 |
Oh, I missed the notification. Thanks for the ping! I'll take a look. |
@braddunbar @Darksonn Gentle ping :) |
@Darksonn Done the improvements |
@braddunbar @Darksonn Ready for another round :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mostly LGTM.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I spent some time combing over this today and I can't find anything to complain about. Thank you @Sytten!
Supersede #687
Closes #611
This is my first time contributing to bytes and this is quite a change, do let me know if you want more tests.