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

Replace the output split key with its pointer in subcompaction #10316

Conversation

littlepig2013
Copy link
Contributor

Summary:

Earlier implementation of cutting the output files with a compact cursor under Round-Robin priority uses Valid() to determine if the output_split_key is valid in ShouldStopBefore. This contributes to excessive CPU computation, as pointed out by this issue. In this PR, we change the type of output_split_key to be InternalKey* and set it as nullptr if it is not going to be used in ShouldStopBefore, Valid() condition checking can be avoided using that pointer.

@littlepig2013 littlepig2013 force-pushed the replace_split_key_with_ptr_in_subcmpct branch from d76b4f3 to f4badd8 Compare July 6, 2022 19:58
@littlepig2013 littlepig2013 requested a review from ajkr July 6, 2022 20:11
Copy link
Contributor

@ajkr ajkr 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! Feel free to import PRs as soon as you create/push them for easier review.

@facebook-github-bot
Copy link
Contributor

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

facebook-github-bot pushed a commit that referenced this pull request Jul 27, 2022
Summary:
Update HISTORY.md for CompactionPri::kRoundRobin. Detailed implementation can be found in [PR10107](#10107), [PR10227](#10227), [PR10250](#10250), [PR10278](#10278), [PR10316](#10316), and [PR10341](#10341)

Pull Request resolved: #10421

Reviewed By: ajkr

Differential Revision: D38194070

Pulled By: littlepig2013

fbshipit-source-id: 4ce153dc0bf22cd865d09c5429955023dbc90f37
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