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

Memory arbitration related code refactor #11168

Closed
wants to merge 1 commit into from

Conversation

xiaoxmeng
Copy link
Contributor

Summary:
Update participant operator methods to better support global arbitration.
Remove stats from existing arbitrator stats which are not used much in production monitoring to simplify code
Add memory arbitration context type to differentiate local arbitration context and global arbitration context. The latter
doesn't have an associated request memory poll as in global arbitration optimization
Rename ScopedMemoryPoolArbitrationContext to MemoryPoolArbitrationSection as we have too many context things
in memory code space
Add metrics used by global arbitration optimization and remove the ones not used much in production monitoring

Differential Revision: D63815963

@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D63815963

@facebook-github-bot facebook-github-bot added CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. fb-exported labels Oct 4, 2024
Copy link

netlify bot commented Oct 4, 2024

Deploy Preview for meta-velox canceled.

Name Link
🔨 Latest commit 136e3a6
🔍 Latest deploy log https://app.netlify.com/sites/meta-velox/deploys/66ff8abcaec2e20008a1b760

xiaoxmeng added a commit to xiaoxmeng/velox that referenced this pull request Oct 4, 2024
Summary:

Update participant operator methods to better support global arbitration.
Remove stats from existing arbitrator stats which are not used much in production monitoring to simplify code
Add memory arbitration context type to differentiate local arbitration context and global arbitration context. The latter
doesn't have an associated request memory poll as in global arbitration optimization
Rename ScopedMemoryPoolArbitrationContext to MemoryPoolArbitrationSection as we have too many context things
in memory code space
Add metrics used by global arbitration optimization and remove the ones not used much in production monitoring

Differential Revision: D63815963
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D63815963

Summary:

Update participant operator methods to better support global arbitration.
Remove stats from existing arbitrator stats which are not used much in production monitoring to simplify code
Add memory arbitration context type to differentiate local arbitration context and global arbitration context. The latter
doesn't have an associated request memory poll as in global arbitration optimization
Rename ScopedMemoryPoolArbitrationContext to MemoryPoolArbitrationSection as we have too many context things
in memory code space
Add metrics used by global arbitration optimization and remove the ones not used much in production monitoring

Reviewed By: tanjialiang

Differential Revision: D63815963
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D63815963

@facebook-github-bot
Copy link
Contributor

This pull request has been merged in 7b6ef14.

@xiaoxmeng xiaoxmeng deleted the export-D63815963 branch October 4, 2024 14:50
Copy link

Conbench analyzed the 1 benchmark run on commit 7b6ef141.

There were no benchmark performance regressions. 🎉

The full Conbench report has more details.

// The time that waits for global arbitration queue.
uint64_t globalArbitrationWaitTimeUs_{0};
// The time that starts global arbitration wait
uint64_t globalArbitrationStartTimeMs_{};
Copy link
Contributor

Choose a reason for hiding this comment

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

init to 0

slowCapacityGrowRatio,
minFreeCapacity,
succinctBytes(minFreeCapacity),
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we add memoryPool back to prefix? It will help with readability. Also could you please make sure to make followup changes to all the same meaning name across the codebase to make it consistent?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. fb-exported Merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants