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

Delete copy and move operations of some Matrix classes #2100

Closed
wants to merge 2 commits into from

Conversation

cyyever
Copy link
Contributor

@cyyever cyyever commented Oct 27, 2023

For those matrices managing their buffers, the copy and move operations should be deleted to avoid default versions generated by the compiler. Another solution is to manage the buffer ownership in user defined versions, but deleting them is simpler given that no memory corruption has been found since it can be inferred that they are not used.

Another solution is use std::share_ptr to manager buffers. However, it will incur some atomic operations on the reference counter, which is not desirable .

@netlify
Copy link

netlify bot commented Oct 27, 2023

Deploy Preview for pytorch-fbgemm-docs canceled.

Name Link
🔨 Latest commit d28636c
🔍 Latest deploy log https://app.netlify.com/sites/pytorch-fbgemm-docs/deploys/654047e1f3e721000799ef8d

@cyyever cyyever changed the title Disable copy operations Disable copy and move operations Oct 27, 2023
@cyyever cyyever changed the title Disable copy and move operations Disable copy and move operations of some Matrix classes Oct 27, 2023
@q10
Copy link
Contributor

q10 commented Oct 30, 2023

@cyyever can I trouble you rebase the branch to top of tree main?

@cyyever
Copy link
Contributor Author

cyyever commented Oct 31, 2023

@q10 Done

@cyyever cyyever changed the title Disable copy and move operations of some Matrix classes Delete copy and move operations of some Matrix classes Oct 31, 2023
@facebook-github-bot
Copy link
Contributor

@q10 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

q10 pushed a commit to q10/FBGEMM that referenced this pull request Oct 31, 2023
Summary:
For those matrices managing their buffers, the copy and move operations should be deleted to avoid default versions generated by the compiler. Another solution is to manage the buffer ownership in user defined versions, but deleting them is simpler given that no memory corruption has been found since it can be inferred that they are not used.

Another solution is use std::share_ptr to manager buffers. However, it will incur some atomic operations on the reference counter, which is not desirable .

Pull Request resolved: pytorch#2100

Reviewed By: spcyppt

Differential Revision: D50822752

Pulled By: q10

fbshipit-source-id: be291b2aeaf19424df3b4aab4cf7feaca8278396
q10 pushed a commit to q10/FBGEMM that referenced this pull request Oct 31, 2023
Summary:
Pull Request resolved: pytorch#2103

For those matrices managing their buffers, the copy and move operations should be deleted to avoid default versions generated by the compiler. Another solution is to manage the buffer ownership in user defined versions, but deleting them is simpler given that no memory corruption has been found since it can be inferred that they are not used.

Another solution is use std::share_ptr to manager buffers. However, it will incur some atomic operations on the reference counter, which is not desirable .

Pull Request resolved: pytorch#2100

Reviewed By: spcyppt

Differential Revision: D50822752

Pulled By: q10

fbshipit-source-id: 05eb6036248b3f19727477bfcaec1f5dc42d9a65
@facebook-github-bot
Copy link
Contributor

@q10 merged this pull request in 488fa2e.

@cyyever cyyever deleted the copy_assignable branch November 1, 2023 00:31
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.

3 participants