This is an automated email from the ASF dual-hosted git repository.
yangzy pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/incubator-gluten.git
The following commit(s) were added to refs/heads/main by this push:
new 6fba94991 [GLUTEN-5673][VL] Fix arbitrator grow logic when exist
concurrent memory request (#5674)
6fba94991 is described below
commit 6fba94991a4232839654edfbcb1668a5df45203c
Author: Yang Zhang <[email protected]>
AuthorDate: Fri May 10 12:47:10 2024 +0800
[GLUTEN-5673][VL] Fix arbitrator grow logic when exist concurrent memory
request (#5674)
---
cpp/velox/memory/VeloxMemoryManager.cc | 35 +++++++++++++++++-----------------
1 file changed, 18 insertions(+), 17 deletions(-)
diff --git a/cpp/velox/memory/VeloxMemoryManager.cc
b/cpp/velox/memory/VeloxMemoryManager.cc
index 49edba4c4..1347bb950 100644
--- a/cpp/velox/memory/VeloxMemoryManager.cc
+++ b/cpp/velox/memory/VeloxMemoryManager.cc
@@ -46,7 +46,7 @@ class ListenableArbitrator : public
velox::memory::MemoryArbitrator {
uint64_t shrinkCapacity(velox::memory::MemoryPool* pool, uint64_t
targetBytes) override {
std::lock_guard<std::recursive_mutex> l(mutex_);
- return releaseMemoryLocked(pool, targetBytes);
+ return shrinkCapacityLocked(pool, targetBytes);
}
bool growCapacity(
@@ -55,10 +55,10 @@ class ListenableArbitrator : public
velox::memory::MemoryArbitrator {
uint64_t targetBytes) override {
VELOX_CHECK_EQ(candidatePools.size(), 1, "ListenableArbitrator should only
be used within a single root pool")
auto candidate = candidatePools.back();
- VELOX_CHECK(pool->root() == candidate.get(), "Illegal state in
ListenableArbitrator") {
- std::lock_guard<std::recursive_mutex> l(mutex_);
- growPoolLocked(pool->root(), targetBytes);
- }
+ VELOX_CHECK(pool->root() == candidate.get(), "Illegal state in
ListenableArbitrator");
+
+ std::lock_guard<std::recursive_mutex> l(mutex_);
+ growCapacityLocked(pool->root(), targetBytes);
return true;
}
@@ -90,28 +90,29 @@ class ListenableArbitrator : public
velox::memory::MemoryArbitrator {
}
private:
- uint64_t growPoolLocked(velox::memory::MemoryPool* pool, uint64_t bytes) {
+ void growCapacityLocked(velox::memory::MemoryPool* pool, uint64_t bytes) {
// Since
//
https://github.com/facebookincubator/velox/pull/9557/files#diff-436e44b7374032f8f5d7eb45869602add6f955162daa2798d01cc82f8725724dL812-L820,
// We should pass bytes as parameter "reservationBytes" when calling
::grow.
- const uint64_t freeBytes = pool->freeBytes();
- if (freeBytes >= bytes) {
- bool reserved = pool->grow(0, bytes);
- GLUTEN_CHECK(
- reserved,
- "Unexpected: Failed to reserve " + std::to_string(bytes) +
- " bytes although there is enough space, free bytes: " +
std::to_string(freeBytes));
- return 0;
+ auto freeByes = pool->freeBytes();
+ if (freeByes > bytes) {
+ if (pool->grow(0, bytes)) {
+ return;
+ }
}
auto reclaimedFreeBytes = pool->shrink(0);
auto neededBytes = bytes - reclaimedFreeBytes;
listener_->allocationChanged(neededBytes);
auto ret = pool->grow(bytes, bytes);
- VELOX_CHECK(ret, "{} failed to grow {} bytes", pool->name(),
velox::succinctBytes(bytes))
- return ret;
+ VELOX_CHECK(
+ ret,
+ "{} failed to grow {} bytes, current state {}",
+ pool->name(),
+ velox::succinctBytes(bytes),
+ pool->toString())
}
- uint64_t releaseMemoryLocked(velox::memory::MemoryPool* pool, uint64_t
bytes) {
+ uint64_t shrinkCapacityLocked(velox::memory::MemoryPool* pool, uint64_t
bytes) {
uint64_t freeBytes = pool->shrink(bytes);
listener_->allocationChanged(-freeBytes);
return freeBytes;
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
