You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by GitBox <gi...@apache.org> on 2020/11/20 21:59:18 UTC

[GitHub] [arrow] alamb commented on a change in pull request #8545: ARROW-9674: [Rust] Make the parquet read and writers Send

alamb commented on a change in pull request #8545:
URL: https://github.com/apache/arrow/pull/8545#discussion_r527993120



##########
File path: rust/parquet/src/util/memory.rs
##########
@@ -39,37 +41,39 @@ pub type WeakMemTrackerPtr = Weak<MemTracker>;
 pub struct MemTracker {
     // In the tuple, the first element is the current memory allocated (in bytes),
     // and the second element is the maximum memory allocated so far (in bytes).
-    memory_usage: Cell<(i64, i64)>,
+    current_memory_usage: AtomicI64,
+    max_memory_usage: AtomicI64,
 }
 
 impl MemTracker {
     /// Creates new memory tracker.
     #[inline]
     pub fn new() -> MemTracker {
         MemTracker {
-            memory_usage: Cell::new((0, 0)),
+            current_memory_usage: Default::default(),
+            max_memory_usage: Default::default(),
         }
     }
 
     /// Returns the current memory consumption, in bytes.
     pub fn memory_usage(&self) -> i64 {
-        self.memory_usage.get().0
+        self.current_memory_usage.load(Ordering::Acquire)
     }
 
     /// Returns the maximum memory consumption so far, in bytes.
     pub fn max_memory_usage(&self) -> i64 {
-        self.memory_usage.get().1
+        self.max_memory_usage.load(Ordering::Acquire)
     }
 
     /// Adds `num_bytes` to the memory consumption tracked by this memory tracker.
     #[inline]
     pub fn alloc(&self, num_bytes: i64) {
-        let (current, mut maximum) = self.memory_usage.get();
-        let new_current = current + num_bytes;
-        if new_current > maximum {
-            maximum = new_current
-        }
-        self.memory_usage.set((new_current, maximum));
+        let new_current = self
+            .current_memory_usage

Review comment:
       👍  This is a nice change to remove the use of `Cell`




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org