refactor: Simplify TabletReader create API and member ownership (#621)#621
Closed
xiaoxmeng wants to merge 2 commits intofacebookincubator:mainfrom
Closed
refactor: Simplify TabletReader create API and member ownership (#621)#621xiaoxmeng wants to merge 2 commits intofacebookincubator:mainfrom
xiaoxmeng wants to merge 2 commits intofacebookincubator:mainfrom
Conversation
meta-cla
bot
added
the
CLA Signed
|
@xiaoxmeng has exported this pull request. If you are a Meta employee, you can view the originating Diff in D98689849. |
tanjialiang
approved these changes
Mar 30, 2026
meta-codesync
bot
changed the title
xiaoxmeng
added a commit
to xiaoxmeng/nimble
that referenced
this pull request
Mar 30, 2026
Summary: CONTEXT: TabletReader had three creation methods: two production `create` overloads (one taking `shared_ptr<ReadFile>`, one taking raw `ReadFile*`) and a `testingCreate` for pre-parsed metadata. The raw pointer overload was error-prone and the testingCreate was unnecessarily public. WHAT: Remove the raw `ReadFile*` create overload and the public `testingCreate` method. Keep only the `shared_ptr<ReadFile>` create method as the single production API. The pre-parsed metadata constructor is now private, accessible only to friend classes (TabletHelper, TabletReaderTestHelper). Updated all callers across the codebase to use the shared_ptr API. Reviewed By: HuamengJiang, tanjialiang Differential Revision: D98689849
xiaoxmeng
force-pushed
the
export-D98689849
branch
2 times, most recently
from
1028779 to
0d431c8
Compare
xiaoxmeng
added a commit
to xiaoxmeng/nimble
that referenced
this pull request
Mar 30, 2026
Summary: Pull Request resolved: facebookincubator#621 CONTEXT: TabletReader had three creation methods: two production `create` overloads (one taking `shared_ptr<ReadFile>`, one taking raw `ReadFile*`) and a `testingCreate` for pre-parsed metadata. The raw pointer overload was error-prone and the testingCreate was unnecessarily public. WHAT: Remove the raw `ReadFile*` create overload and the public `testingCreate` method. Keep only the `shared_ptr<ReadFile>` create method as the single production API. The pre-parsed metadata constructor is now private, accessible only to friend classes (TabletHelper, TabletReaderTestHelper). Updated all callers across the codebase to use the shared_ptr API. Reviewed By: HuamengJiang, tanjialiang Differential Revision: D98689849
xiaoxmeng
added a commit
to xiaoxmeng/nimble
that referenced
this pull request
Mar 30, 2026
Summary: CONTEXT: TabletReader had three creation methods: two production `create` overloads (one taking `shared_ptr<ReadFile>`, one taking raw `ReadFile*`) and a `testingCreate` for pre-parsed metadata. The raw pointer overload was error-prone and the testingCreate was unnecessarily public. WHAT: Remove the raw `ReadFile*` create overload and the public `testingCreate` method. Keep only the `shared_ptr<ReadFile>` create method as the single production API. The pre-parsed metadata constructor is now private, accessible only to friend classes (TabletHelper, TabletReaderTestHelper). Updated all callers across the codebase to use the shared_ptr API. Reviewed By: HuamengJiang, tanjialiang Differential Revision: D98689849
xiaoxmeng
force-pushed
the
export-D98689849
branch
from
0d431c8 to
8598e6d
Compare
xiaoxmeng
added a commit
to xiaoxmeng/nimble
that referenced
this pull request
Mar 30, 2026
Summary: Pull Request resolved: facebookincubator#621 CONTEXT: TabletReader had three creation methods: two production `create` overloads (one taking `shared_ptr<ReadFile>`, one taking raw `ReadFile*`) and a `testingCreate` for pre-parsed metadata. The raw pointer overload was error-prone and the testingCreate was unnecessarily public. WHAT: Remove the raw `ReadFile*` create overload and the public `testingCreate` method. Keep only the `shared_ptr<ReadFile>` create method as the single production API. The pre-parsed metadata constructor is now private, accessible only to friend classes (TabletHelper, TabletReaderTestHelper). Updated all callers across the codebase to use the shared_ptr API. Reviewed By: HuamengJiang, tanjialiang Differential Revision: D98689849
xiaoxmeng
force-pushed
the
export-D98689849
branch
from
8598e6d to
fa3b6ae
Compare
Summary: CONTEXT: Nimble OSS build tracks a specific Velox commit for its submodule dependency. WHAT: Advance Velox submodule pin from a10e28640c to 6566eba401dc. Differential Revision: D98697953
meta-codesync
bot
changed the title
xiaoxmeng
force-pushed
the
export-D98689849
branch
from
fa3b6ae to
0295d40
Compare
…bookincubator#621) Summary: Pull Request resolved: facebookincubator#621 CONTEXT: TabletReader had three creation methods: two production `create` overloads (one taking `shared_ptr<ReadFile>`, one taking raw `ReadFile*`) and a `testingCreate` for pre-parsed metadata. The raw pointer overload was error-prone and the testingCreate was unnecessarily public. Internally, file ownership was split across two members (`file_` raw pointer and `ownedFile_` shared_ptr), and metadata input had a redundant raw pointer alias (`metadataBufferedInput_`). WHAT: - Remove the raw `ReadFile*` create overload and the public `testingCreate` method. Keep only the `shared_ptr<ReadFile>` create method as the single production API. - Remove the pre-parsed metadata constructor. The private constructor now takes only `(shared_ptr<ReadFile>, MemoryPool&, Options&)`. - Merge `file_` (raw pointer) and `ownedFile_` (shared_ptr) into a single `shared_ptr<ReadFile> file_` member. - Remove redundant `metadataBufferedInput_` raw pointer alias; use `metadataInput_` unique_ptr directly. - Update FileLayout::create to accept `shared_ptr<ReadFile>`. - Updated all callers across the codebase to use the shared_ptr API. Reviewed By: HuamengJiang, tanjialiang Differential Revision: D98689849
meta-codesync
bot
changed the title
xiaoxmeng
force-pushed
the
export-D98689849
branch
from
0295d40 to
e6ae881
Compare
|
This pull request has been merged in 552bea6. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary:
CONTEXT: TabletReader had three creation methods: two production
createoverloads (one takingshared_ptr<ReadFile>, one taking rawReadFile*) and atestingCreatefor pre-parsed metadata. The raw pointer overload was error-prone and the testingCreate was unnecessarily public. Internally, file ownership was split across two members (file_raw pointer andownedFile_shared_ptr), and metadata input had a redundant raw pointer alias (metadataBufferedInput_).WHAT:
ReadFile*create overload and the publictestingCreatemethod. Keep only theshared_ptr<ReadFile>create method as the single production API.(shared_ptr<ReadFile>, MemoryPool&, Options&).file_(raw pointer) andownedFile_(shared_ptr) into a singleshared_ptr<ReadFile> file_member.metadataBufferedInput_raw pointer alias; usemetadataInput_unique_ptr directly.shared_ptr<ReadFile>.Reviewed By: HuamengJiang, tanjialiang
Differential Revision: D98689849