Simplify the memory pool arbitration process management #10705
Conversation
✅ Deploy Preview for meta-velox canceled.
|
95a65dc to
43eadab
Compare
|
@xiaoxmeng has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
|
Title -> s/Simply/Simplify/ |
| @@ -165,7 +165,6 @@ class SharedArbitrator : public memory::MemoryArbitrator { | |||
| // Contains the execution state of an arbitration operation. | |||
| struct ArbitrationOperation { | |||
| MemoryPool* const requestPool; | |||
| MemoryPool* const requestRoot; | |||
There was a problem hiding this comment.
Should we keep the name requestRoot instead of requestPool? Will be more intuitive and provides better readability
There was a problem hiding this comment.
The request root is named to be different than the leaf pool. Now we always initiate from a root memory pool so we don't need to name it as root.
| @@ -425,6 +425,18 @@ class ScopedMemoryArbitrationContext { | |||
| MemoryArbitrationContext currentArbitrationCtx_; | |||
| }; | |||
|
|
|||
| /// Object used to setup memory arbitration context for a leaf memory pool. | |||
| class ScopedMemoryArbitration { | |||
There was a problem hiding this comment.
Looking together with the above class, plus a scoped class containing another scoped class, it is quite confusing. Can we remove ScopedMemoryArbitrationContext class and put the impl of it inside of ScopedMemoryArbitration class?
There was a problem hiding this comment.
Chat offline. They are different.
|
@xiaoxmeng has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
|
This pull request was exported from Phabricator. Differential Revision: D61063144 |
…bator#10705) Summary: Move memory arbitration context setting from shared arbitrator to memory pool and pass the root memory pool to memory arbitrator directly to simplify the the arbitrator backend implementation. The memory pool handles the memory arbitration context setting and grow the capacity with the memory arbitrator directly. Pull Request resolved: facebookincubator#10705 Reviewed By: tanjialiang Differential Revision: D61063144 Pulled By: xiaoxmeng
|
This pull request was exported from Phabricator. Differential Revision: D61063144 |
…bator#10705) Summary: Move memory arbitration context setting from shared arbitrator to memory pool and pass the root memory pool to memory arbitrator directly to simplify the the arbitrator backend implementation. The memory pool handles the memory arbitration context setting and grow the capacity with the memory arbitrator directly. Pull Request resolved: facebookincubator#10705 Reviewed By: tanjialiang Differential Revision: D61063144 Pulled By: xiaoxmeng
|
This pull request was exported from Phabricator. Differential Revision: D61063144 |
…bator#10705) Summary: Move memory arbitration context setting from shared arbitrator to memory pool and pass the root memory pool to memory arbitrator directly to simplify the the arbitrator backend implementation. The memory pool handles the memory arbitration context setting and grow the capacity with the memory arbitrator directly. Pull Request resolved: facebookincubator#10705 Reviewed By: tanjialiang Differential Revision: D61063144 Pulled By: xiaoxmeng
…bator#10705) Summary: Move memory arbitration context setting from shared arbitrator to memory pool and pass the root memory pool to memory arbitrator directly to simplify the the arbitrator backend implementation. The memory pool handles the memory arbitration context setting and grow the capacity with the memory arbitrator directly. Pull Request resolved: facebookincubator#10705 Reviewed By: tanjialiang Differential Revision: D61063144 Pulled By: xiaoxmeng
|
This pull request was exported from Phabricator. Differential Revision: D61063144 |
…bator#10705) Summary: Move memory arbitration context setting from shared arbitrator to memory pool and pass the root memory pool to memory arbitrator directly to simplify the the arbitrator backend implementation. The memory pool handles the memory arbitration context setting and grow the capacity with the memory arbitrator directly. Pull Request resolved: facebookincubator#10705 Reviewed By: tanjialiang Differential Revision: D61063144 Pulled By: xiaoxmeng
|
This pull request was exported from Phabricator. Differential Revision: D61063144 |
|
@xiaoxmeng merged this pull request in 5f32870. |
|
Conbench analyzed the 1 benchmark run on commit There were no benchmark performance regressions. 🎉 The full Conbench report has more details. |
Move memory arbitration context setting from shared arbitrator to memory pool and pass the
root memory pool to memory arbitrator directly to simplify the the arbitrator backend implementation.
The memory pool handles the memory arbitration context setting and grow the capacity with the
memory arbitrator directly.