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

file_system.wrapper.remove.redudant.fields #10545

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

rockeet
Copy link
Contributor

@rockeet rockeet commented Aug 20, 2022

Remove redundant field.

@mrambacher
Copy link
Contributor

I am confused as to the advantage of this change. The existing code uses a unique ptr to store the moved unique ptr. The proposal uses a raw pointer to accomplish the same result.

Is there a reason the raw approach is better than the unique ptr one? I believe there are very few unguarded pointers left in RocksDB intentionally.

@rockeet
Copy link
Contributor Author

rockeet commented Aug 21, 2022

I am confused as to the advantage of this change. The existing code uses a unique ptr to store the moved unique ptr. The proposal uses a raw pointer to accomplish the same result.

Is there a reason the raw approach is better than the unique ptr one? I believe there are very few unguarded pointers left in RocksDB intentionally.

The raw ptr target_ is already defined in base class, the unique_ptr is a new data field in derived class which consume memory and is just a copy of target_ in base class.

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.

None yet

3 participants