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


##########
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());

Review Comment:
   If the array as a length of zero, `MemoryMeter.measureArray` will return the 
array based offset which is an empty array size. You avoid the miss-prediction 
cost for the if, as zero size array are rare, and get the same result. + it is 
easier to read.   



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