yjshen commented on a change in pull request #1691:
URL: https://github.com/apache/arrow-datafusion/pull/1691#discussion_r795044951
##########
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:
I think fetch_update won't let infinite loop here. A simple case zero
minus ten results in underflow:
https://play.rust-lang.org/?version=stable&mode=release&edition=2021
I agree the mutex version is easier to write and reason. I can revert the
last commit if preferred.
--
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
