Skip to content

feat(bindings/cpp): eliminate indirect pointers by manually managing memory #6147

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

Merged
merged 1 commit into from
May 6, 2025

Conversation

deadlinefen
Copy link
Contributor

Eliminate indirect pointers by manually managing the memory of returned classes in lib.rs

Which issue does this PR close?

nop.

Rationale for this change

The previous type erasure implementation using unique_ptr introduced an indirection pointer, resulting in additional overhead.
This was previously mentioned in the earlier PR #6041.

What changes are included in this PR?

  1. Modifying lib.rs to return raw pointers instead of Box<T> removes one layer of erasure overhead, transferring memory management of the returned class from the Rust layer to the C++ layer.
  2. Extract class definitions such as Metadata and Entry from opendal.hpp into data_structure.hpp to reduce precompilation overhead from header file expansion.
  3. Modified the file search approach in CMakeLists to prepare for subsequent changes. (BTW, ​we plan to​ reorganize the cpp binding directory structure, as #include "opendal/opendal.hpp" better aligns with modern C++ project conventions than #include "opendal.hpp")

Are there any user-facing changes?

nop.

cc @JackDrogon

@deadlinefen deadlinefen requested a review from Xuanwo as a code owner May 5, 2025 07:55
@dosubot dosubot bot added size:XL This PR changes 500-999 lines, ignoring generated files. releases-note/feat The PR implements a new feature or has a title that begins with "feat" labels May 5, 2025
…he memory of returned classes in lib.rs

Co-authored-by: JackDrogon <[email protected]>
@deadlinefen deadlinefen force-pushed the remove-double-pointer branch from b603b17 to 7796b72 Compare May 5, 2025 07:58
@Xuanwo Xuanwo changed the title feat(bindings/cpp) eliminate indirect pointers by manually managing t… feat(bindings/cpp): eliminate indirect pointers by manually managing memory May 6, 2025
@Xuanwo Xuanwo requested a review from silver-ymz May 6, 2025 03:12
@Xuanwo
Copy link
Member

Xuanwo commented May 6, 2025

Inviting @silver-ymz and @xyjixyjixyji for a review.

Copy link
Member

@silver-ymz silver-ymz left a comment

Choose a reason for hiding this comment

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

LGTM, Thanks.

@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label May 6, 2025
@Xuanwo Xuanwo merged commit 193a30e into apache:main May 6, 2025
33 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lgtm This PR has been approved by a maintainer releases-note/feat The PR implements a new feature or has a title that begins with "feat" size:XL This PR changes 500-999 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants