blerer commented on code in PR #2488:
URL: https://github.com/apache/cassandra/pull/2488#discussion_r1265065053


##########
src/java/org/apache/cassandra/utils/ObjectSizes.java:
##########
@@ -134,65 +124,89 @@ public static long sizeOnHeapOf(ByteBuffer[] array)
 
     /**
      * 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)}.
+     * by the array itself and for each included byte buffer using {@link 
#sizeOnHeapExcludingDataOf(ByteBuffer)}.
      */
-    public static long sizeOnHeapExcludingData(ByteBuffer[] array)
+    public static long sizeOnHeapExcludingDataOf(ByteBuffer[] array)
     {
         if (array == null)
             return 0;
 
         long sum = sizeOfArray(array);
         for (ByteBuffer b : array)
-            sum += sizeOnHeapExcludingData(b);
+            sum += sizeOnHeapExcludingDataOf(b);
 
         return sum;
     }
 
     /**
-     * @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.
+     * Measures the heap memory used by the specified byte buffer. If the 
buffer is a slab only the data size will be
+     * counted but not the internal overhead. A SLAB is assumed to be created 
by: {@code buffer.duplicate().position(start).limit(end)} without the use of 
{@code slice()}.
+     * <p>This method makes a certain amount of assumptions:
+     *   <ul>
+     *       <li>That slabs are always created using: {@code 
buffer.duplicate().position(start).limit(end)} and not through slice</li>
+     *       <li>That the input buffers are not read-only buffers</li>
+     *       <li>That the direct buffers that are not slab are not 
duplicates</li>  
+     *   </ul>
+     *  Non-respect of those assumptions can lead to an invalid value being 
returned.
+     * @param buffer the buffer to measure
+     * @return the heap memory used by the specified byte buffer.
      */
     public static long sizeOnHeapOf(ByteBuffer buffer)
     {
         if (buffer == null)
             return 0;
 
-        if (buffer.isDirect())
-            return DIRECT_BUFFER_HEAP_SIZE;
+        assert !buffer.isReadOnly();
 
-        int arrayLen = buffer.array().length;
-        int bufLen = buffer.remaining();
+        // We assume here that slabs are always created using: 
buffer.duplicate().position(start).limit(end) and not through slice
+        if (ByteBufferMode.SLAB_ALLOCATION_NO_SLICE.isSlab(buffer))
+        {
+            if (buffer.isDirect())
+                return DIRECT_BUFFER_SHALLOW_SIZE; // We ignore the underlying 
buffer
+
+            return HEAP_BUFFER_SHALLOW_SIZE + buffer.remaining(); // We ignore 
the array overhead
+        }
 
-        // 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;
+        if (buffer.isDirect())
+            return DIRECT_BUFFER_DEEP_SIZE; // That might not be true if the 
buffer is a view of another buffer so we could undercount
 
-        return EMPTY_HEAP_BUFFER_SIZE + (arrayLen == 0 ? EMPTY_BYTE_ARRAY_SIZE 
: sizeOfArray(arrayLen, 1));
+        return HEAP_BUFFER_SHALLOW_SIZE + meter.measureArray(buffer.array());
     }
 
     /**
-     * @return non-data heap memory consumed by the byte buffer. If it is a 
slice, it does not include the internal
-     * array overhead.
+     * Measures the heap memory used by the specified byte buffer excluding 
the data. If the buffer shallow size will be counted.
+     * A SLAB is assumed to be created by: {@code 
buffer.duplicate().position(start).limit(end)} without the use of {@code 
slice()}.
+     * <p>This method makes a certain amount of assumptions:
+     *   <ul>
+     *       <li>That slabs are always created using: {@code 
buffer.duplicate().position(start).limit(end)} and not through slice</li>
+     *       <li>That the input buffers are not read-only buffers</li>
+     *       <li>That the direct buffers that are not slab are not 
duplicates</li>  
+     *   </ul>
+     *  Non-respect of those assumptions can lead to an invalid value being 
returned. T 
+     * @param buffer the buffer to measure
+     * @return the heap memory used by the specified byte buffer excluding the 
data..
      */
-    public static long sizeOnHeapExcludingData(ByteBuffer buffer)
+    public static long sizeOnHeapExcludingDataOf(ByteBuffer buffer)
     {
         if (buffer == null)
             return 0;
 
-        if (buffer.isDirect())
-            return DIRECT_BUFFER_HEAP_SIZE;
+        assert !buffer.isReadOnly();
 
-        int arrayLen = buffer.array().length;
-        int bufLen = buffer.remaining();
+        // We assume here that slabs are always created using: 
buffer.duplicate().position(start).limit(end) and not through slice
+        if (ByteBufferMode.SLAB_ALLOCATION_NO_SLICE.isSlab(buffer))
+        {
+            if (buffer.isDirect())
+                return DIRECT_BUFFER_SHALLOW_SIZE; // We ignore the underlying 
buffer
 
-        // 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;
+            return HEAP_BUFFER_SHALLOW_SIZE; // We ignore the array overhead
+        }
+
+        if (buffer.isDirect())
+            return DIRECT_BUFFER_DEEP_SIZE; // That might not be true if the 
buffer is a view of another buffer so we could undercount
 
-        // 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));
+        byte[] bytes = buffer.array();
+        return HEAP_BUFFER_SHALLOW_SIZE + meter.measureArray(bytes) - 
bytes.length;

Review Comment:
   Same answer ;-) 



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


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to