Skip to content

Remove IMemoryManager interface#5703

Closed
tanjialiang wants to merge 1 commit intofacebookincubator:mainfrom
tanjialiang:rm_imgr
Closed

Remove IMemoryManager interface#5703
tanjialiang wants to merge 1 commit intofacebookincubator:mainfrom
tanjialiang:rm_imgr

Conversation

@tanjialiang
Copy link
Contributor

Removing the interface as there is no use case needed for this interface

@tanjialiang tanjialiang requested a review from xiaoxmeng July 18, 2023 18:46
@netlify
Copy link

netlify bot commented Jul 18, 2023

Deploy Preview for meta-velox canceled.

Name Link
🔨 Latest commit 1aa0941
🔍 Latest deploy log https://app.netlify.com/sites/meta-velox/deploys/64b84d86bb4ff4000806ce42

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Jul 18, 2023
@facebook-github-bot
Copy link
Contributor

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

Copy link
Contributor

@xiaoxmeng xiaoxmeng left a comment

Choose a reason for hiding this comment

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

@tanjialiang thanks for the refactor!


/// Tries to get the singleton memory manager. If not previously initialized,
/// the process singleton manager will be initialized with the given capacity.
FOLLY_EXPORT static MemoryManager& getInstance(
Copy link
Contributor

Choose a reason for hiding this comment

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

Move this to cc? thanks!

const std::string& name = "",
int64_t capacity = kMaxMemory,
std::unique_ptr<MemoryReclaimer> reclaimer = nullptr) = 0;
int64_t maxBytes = kMaxMemory,
Copy link
Contributor

Choose a reason for hiding this comment

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

Let' call this capacity?

@facebook-github-bot
Copy link
Contributor

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

@facebook-github-bot
Copy link
Contributor

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

@facebook-github-bot
Copy link
Contributor

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

@facebook-github-bot
Copy link
Contributor

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

tanjialiang added a commit to tanjialiang/velox-1 that referenced this pull request Jul 19, 2023
Summary:
Removing the interface as there is no use case needed for this interface

Pull Request resolved: facebookincubator#5703

Reviewed By: xiaoxmeng

Differential Revision: D47560079

Pulled By: tanjialiang

fbshipit-source-id: effe66d11d0a142f86952a5476b5cb46b122e6ff
@facebook-github-bot
Copy link
Contributor

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

tanjialiang added a commit to tanjialiang/velox-1 that referenced this pull request Jul 19, 2023
Summary:
Removing the interface as there is no use case needed for this interface

Pull Request resolved: facebookincubator#5703

Reviewed By: xiaoxmeng

Differential Revision: D47560079

Pulled By: tanjialiang

fbshipit-source-id: d4a9e678b6cb4b37640a0fd86cf535520fa6a8ec
@facebook-github-bot
Copy link
Contributor

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

Summary:
Removing the interface as there is no use case needed for this interface

Pull Request resolved: facebookincubator#5703

Reviewed By: xiaoxmeng

Differential Revision: D47560079

Pulled By: tanjialiang

fbshipit-source-id: f07acbb51caf81b9f950e27a1bbcba72e9f17b52
@facebook-github-bot
Copy link
Contributor

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

@facebook-github-bot
Copy link
Contributor

@tanjialiang merged this pull request in fe3d937.

@conbench-facebook
Copy link

Conbench analyzed the 1 benchmark run on commit fe3d937d.

There were no benchmark performance regressions. 🎉

The full Conbench report has more details.

tanjialiang added a commit to tanjialiang/presto that referenced this pull request Jul 20, 2023
Summary:
Removing the interface as there is no use case needed for this interface

X-link: facebookincubator/velox#5703

Reviewed By: xiaoxmeng

Differential Revision: D47560079

Pulled By: tanjialiang

fbshipit-source-id: f6bd797b780dcadbc7768bde9dba2317518a259a
tanjialiang added a commit to prestodb/presto that referenced this pull request Jul 20, 2023
Summary:
Removing the interface as there is no use case needed for this interface

X-link: facebookincubator/velox#5703

Reviewed By: xiaoxmeng

Differential Revision: D47560079

Pulled By: tanjialiang

fbshipit-source-id: f6bd797b780dcadbc7768bde9dba2317518a259a
wypb pushed a commit to wypb/presto that referenced this pull request Dec 22, 2023
Summary:
Removing the interface as there is no use case needed for this interface

X-link: facebookincubator/velox#5703

Reviewed By: xiaoxmeng

Differential Revision: D47560079

Pulled By: tanjialiang

fbshipit-source-id: f6bd797b780dcadbc7768bde9dba2317518a259a
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. Merged

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants