[GitHub] [cassandra] jacek-lewandowski commented on a change in pull request #1470: CASSANDRA-17402: Fix ObjectSizes implementation and usages

[GitBox <gi...@apache.org>]

jacek-lewandowski commented on a change in pull request #1470:
URL: https://github.com/apache/cassandra/pull/1470#discussion_r818660517



##########
File path: src/java/org/apache/cassandra/utils/ObjectSizes.java
##########
@@ -96,58 +112,97 @@ private static long sizeOfArray(int length, long elementSize)
     }
 
     /**
-     * Memory a ByteBuffer array consumes.
+     * Amount of heap memory consumed by the array of byte buffers. It sums memory consumed by the array itself
+     * and for each included byte buffer using {@link #sizeOnHeapOf(ByteBuffer)}.
      */
     public static long sizeOnHeapOf(ByteBuffer[] array)
     {
-        long allElementsSize = 0;
-        for (int i = 0; i < array.length; i++)
-            if (array[i] != null)
-                allElementsSize += sizeOnHeapOf(array[i]);
+        if (array == null)
+            return 0;
 
-        return allElementsSize + sizeOfArray(array);
+        long sum = sizeOfArray(array);
+        for (ByteBuffer buffer : array)
+            sum += sizeOnHeapOf(buffer);
+
+        return sum;
     }
 
+    /**
+     * Amount of non-data heap memory consumed by the array of byte buffers. It sums memory consumed
+     * by the array itself and for each included byte buffer using {@link #sizeOnHeapExcludingData(ByteBuffer)}.
+     */
     public static long sizeOnHeapExcludingData(ByteBuffer[] array)
     {
-        return BUFFER_EMPTY_SIZE * array.length + sizeOfArray(array);
+        if (array == null)
+            return 0;
+
+        long sum = sizeOfArray(array);
+        for (ByteBuffer b : array)
+            sum += sizeOnHeapExcludingData(b);
+
+        return sum;
     }
 
     /**
-     * Memory a byte buffer consumes
-     * @param buffer ByteBuffer to calculate in memory size
-     * @return Total in-memory size of the byte buffer
+     * @return heap memory consumed by the byte buffer. If it is a slice, it counts the data size, but it does not
+     * include the internal array overhead.
      */
     public static long sizeOnHeapOf(ByteBuffer buffer)
     {
+        if (buffer == null)
+            return 0;
+
         if (buffer.isDirect())
-            return BUFFER_EMPTY_SIZE;
-        // if we're only referencing a sub-portion of the ByteBuffer, don't count the array overhead (assume it's slab
-        // allocated, so amortized over all the allocations the overhead is negligible and better to undercount than over)
-        if (buffer.capacity() > buffer.remaining())
-            return buffer.remaining();
-        return BUFFER_EMPTY_SIZE + sizeOfArray(buffer.capacity(), 1);
-    }
+            return DIRECT_BUFFER_HEAP_SIZE;
 
-    public static long sizeOfEmptyHeapByteBuffer()
-    {
-        return BUFFER_EMPTY_SIZE;
+        int arrayLen = buffer.array().length;
+        int bufLen = buffer.remaining();
+
+        // if we're only referencing a sub-portion of the ByteBuffer, don't count the array overhead (assume it is SLAB
+        // allocated - the overhead amortized over all the allocations is negligible and better to undercount than over)
+        if (arrayLen > bufLen)
+            return EMPTY_HEAP_BUFFER_SIZE + bufLen;
+
+        return EMPTY_HEAP_BUFFER_SIZE + (arrayLen == 0 ? EMPTY_BYTE_ARRAY_SIZE : sizeOfArray(arrayLen, 1));
     }
 
-    public static long sizeOfEmptyByteArray()
+    /**
+     * @return non-data heap memory consumed by the byte buffer. If it is a slice, it does not include the internal
+     * array overhead.
+     */
+    public static long sizeOnHeapExcludingData(ByteBuffer buffer)
     {
-        return BYTE_ARRAY_EMPTY_SIZE;
+        if (buffer == null)
+            return 0;
+
+        if (buffer.isDirect())
+            return DIRECT_BUFFER_HEAP_SIZE;
+
+        int arrayLen = buffer.array().length;
+        int bufLen = buffer.remaining();
+
+        // if we're only referencing a sub-portion of the ByteBuffer, don't count the array overhead (assume it is SLAB
+        // allocated - the overhead amortized over all the allocations is negligible and better to undercount than over)
+        if (arrayLen > bufLen)
+            return EMPTY_HEAP_BUFFER_SIZE;
+
+        // If buffers are dedicated, account for byte array size and any padding overhead
+        return EMPTY_HEAP_BUFFER_SIZE + (arrayLen == 0 ? EMPTY_BYTE_ARRAY_SIZE : (sizeOfArray(arrayLen, 1) - arrayLen));
     }
 
     /**
      * Memory a String consumes
+     *
      * @param str String to calculate memory size of
      * @return Total in-memory size of the String
      */
-    //@TODO hard coding this to 2 isn't necessarily correct in Java 11
+    // TODO hard coding this to 2 isn't necessarily correct in Java 11

Review comment:
       yes, I didn't change that logic at all




-- 
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: pr-unsubscribe@cassandra.apache.org

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



---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org