alamb commented on a change in pull request #1691:
URL: https://github.com/apache/arrow-datafusion/pull/1691#discussion_r795040458
##########
File path: datafusion/src/execution/memory_manager.rs
##########
@@ -270,25 +270,35 @@ impl MemoryManager {
requesters: Arc::new(Mutex::new(HashSet::new())),
pool_size,
requesters_total: Arc::new(Mutex::new(0)),
- trackers_total: Arc::new(Mutex::new(0)),
+ trackers_total: AtomicUsize::new(0),
cv: Condvar::new(),
})
}
}
}
fn get_tracker_total(&self) -> usize {
- *self.trackers_total.lock().unwrap()
+ self.trackers_total.load(Ordering::SeqCst)
}
pub(crate) fn grow_tracker_usage(&self, delta: usize) {
- *self.trackers_total.lock().unwrap() += delta;
+ self.trackers_total.fetch_add(delta, Ordering::SeqCst);
}
pub(crate) fn shrink_tracker_usage(&self, delta: usize) {
- let mut total = self.trackers_total.lock().unwrap();
- assert!(*total >= delta);
- *total -= delta;
+ let update =
+ self.trackers_total
+ .fetch_update(Ordering::SeqCst, Ordering::SeqCst, |x| {
Review comment:
Would this potentially end up in an infinite loop if `x` is zero as the
closure would always return None?
After seeing this way, I think I agree that the original implementation
using `Mutex` was better -- sorry about that @yjshen I didn't realize the
subtleties involved here
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscr...@arrow.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
