This is an automated email from the ASF dual-hosted git repository. dcapwell pushed a commit to branch cassandra-4.0 in repository https://gitbox.apache.org/repos/asf/cassandra.git
The following commit(s) were added to refs/heads/cassandra-4.0 by this push: new 979ab72 ArrayClustering.unsharedHeapSize does not include the data so undercounts the heap size 979ab72 is described below commit 979ab72f4f2afe4a23654572cd804184fc0e2089 Author: David Capwell <dcapw...@apache.org> AuthorDate: Wed Aug 11 12:36:38 2021 -0700 ArrayClustering.unsharedHeapSize does not include the data so undercounts the heap size patch by David Capwell; reviewed by Blake Eggleston for CASSANDRA-16845 --- CHANGES.txt | 1 + .../org/apache/cassandra/db/ArrayClustering.java | 6 +- .../cassandra/db/ClusteringHeapSizeTest.java | 84 ++++++++++++++++++++++ 3 files changed, 90 insertions(+), 1 deletion(-) diff --git a/CHANGES.txt b/CHANGES.txt index 5b402f3..d0bdbe9 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,4 +1,5 @@ 4.0.1 + * ArrayClustering.unsharedHeapSize does not include the data so undercounts the heap size (CASSANDRA-16845) * Improve help, doc and error messages about sstabledump -k and -x arguments (CASSANDRA-16818) * Add repaired/unrepaired bytes back to nodetool (CASSANDRA-15282) * Upgrade lz4-java to 1.8.0 to add RH6 support back (CASSANDRA-16753) diff --git a/src/java/org/apache/cassandra/db/ArrayClustering.java b/src/java/org/apache/cassandra/db/ArrayClustering.java index 0d3de49..a6ee991 100644 --- a/src/java/org/apache/cassandra/db/ArrayClustering.java +++ b/src/java/org/apache/cassandra/db/ArrayClustering.java @@ -31,7 +31,11 @@ public class ArrayClustering extends AbstractArrayClusteringPrefix implements Cl public long unsharedHeapSize() { - return EMPTY_SIZE + ObjectSizes.sizeOfArray(values) + values.length; + long arrayRefSize = ObjectSizes.sizeOfArray(values); + long elementsSize = 0; + for (int i = 0; i < values.length; i++) + elementsSize += ObjectSizes.sizeOfArray(values[i]); + return EMPTY_SIZE + arrayRefSize + elementsSize; } public long unsharedHeapSizeExcludingData() diff --git a/test/unit/org/apache/cassandra/db/ClusteringHeapSizeTest.java b/test/unit/org/apache/cassandra/db/ClusteringHeapSizeTest.java new file mode 100644 index 0000000..54d0ce1 --- /dev/null +++ b/test/unit/org/apache/cassandra/db/ClusteringHeapSizeTest.java @@ -0,0 +1,84 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.cassandra.db; + +import java.nio.ByteBuffer; +import java.util.Arrays; +import java.util.Collection; +import java.util.concurrent.CompletableFuture; + +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; + +import org.apache.cassandra.utils.ObjectSizes; +import org.apache.cassandra.utils.concurrent.OpOrder; +import org.apache.cassandra.utils.memory.NativePool; +import org.assertj.core.api.Assertions; + +@RunWith(Parameterized.class) +public class ClusteringHeapSizeTest +{ + private final Clustering<?> clustering; + + public ClusteringHeapSizeTest(Clustering<?> clustering) + { + this.clustering = clustering; + } + + @Test + public void unsharedHeap() + { + long measureDeep = ObjectSizes.measureDeep(clustering); + if (clustering instanceof BufferClustering) + { + // jamm (used in measureDeep) uses .remaining() where as .sizeOnHeapOf() done in unsharedHeapSize actually looks at memory cost + // without assuming the array is shared (unless capacity > remaining); so account for that + measureDeep += ObjectSizes.measureDeep(new byte[0]); + } + long unsharedHeapSize = clustering.unsharedHeapSize(); + + double allowedDiff = 0.1; // 10% is seen as "close enough" + Assertions.assertThat(unsharedHeapSize) + .describedAs("Real size is %d", measureDeep) + .isBetween((long) (measureDeep * (1 - allowedDiff)), (long) (measureDeep * (1 + allowedDiff))); + } + + @Test + public void unsharedHeapSizeExcludingDataLTEUnsharedHeapSize() + { + long max = clustering.unsharedHeapSize(); + long min = clustering.unsharedHeapSizeExcludingData(); + Assertions.assertThat(min).isLessThanOrEqualTo(max); + } + + @Parameterized.Parameters(name = "{0}") + public static Collection<Object[]> data() { + byte[] rawBytes = { 0, 1, 2, 3, 4, 5, 6 }; + NativePool pool = new NativePool(1024, 1024, 1, () -> CompletableFuture.completedFuture(true)); + OpOrder order = new OpOrder(); + + ArrayClustering array = ArrayClustering.make(rawBytes); + BufferClustering buffer = BufferClustering.make(ByteBuffer.wrap(rawBytes)); + return Arrays.asList(new Object[][] { + { array }, + { buffer }, + { new NativeClustering(pool.newAllocator(), order.getCurrent(), array)} + }); + } +} \ No newline at end of file --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org