etseidl commented on code in PR #8898:
URL: https://github.com/apache/arrow-rs/pull/8898#discussion_r2547767386


##########
parquet/src/file/metadata/memory.rs:
##########
@@ -95,21 +95,16 @@ impl<K: HeapSize, V: HeapSize> HeapSize for HashMap<K, V> {
 
 impl<T: HeapSize> HeapSize for Arc<T> {
     fn heap_size(&self) -> usize {
-        // Arc stores weak and strong counts on the heap alongside an instance 
of T
-        2 * std::mem::size_of::<usize>() + std::mem::size_of::<T>() + 
self.as_ref().heap_size()
-    }
-}
-
-impl HeapSize for Arc<dyn HeapSize> {
-    fn heap_size(&self) -> usize {
-        2 * std::mem::size_of::<usize>()
-            + std::mem::size_of_val(self.as_ref())
-            + self.as_ref().heap_size()
+        // Do not count the size of the Arc as that is accounted for by the
+        // caller (the object that contains the Arc).
+        self.as_ref().heap_size()

Review Comment:
   With this change:
   
   ```rust
       let v = vec![0i32; 20];
       println!("filled vec heap size {}", v.heap_size());
       println!("size of vec {}", std::mem::size_of::<Vec<i32>>());
   
       let av = Arc::new(v);
       println!("arc<vec> heap size {}", av.heap_size());
   ```
   
   ```
   filled vec heap size 80
   size of vec 24
   arc<vec> heap size 80
   ```
   
   The `Vec` is now on the heap, but that is not accounted for. 



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to