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 ca4f6b8
org.apache.cassandra.db.rows.ArrayCell#unsharedHeapSizeExcludingData includes
data twice
ca4f6b8 is described below
commit ca4f6b80563aeb3614fe3bce47e4fb620a8f0e8e
Author: David Capwell <[email protected]>
AuthorDate: Mon Aug 30 16:37:53 2021 -0700
org.apache.cassandra.db.rows.ArrayCell#unsharedHeapSizeExcludingData
includes data twice
patch by David Capwell; reviewed by Caleb Rackliffe for CASSANDRA-16900
---
CHANGES.txt | 1 +
.../apache/cassandra/cql3/ColumnIdentifier.java | 2 +-
.../org/apache/cassandra/db/rows/ArrayCell.java | 2 +-
.../org/apache/cassandra/db/rows/BufferCell.java | 6 +-
.../org/apache/cassandra/db/rows/CellPath.java | 2 +-
.../org/apache/cassandra/db/rows/NativeCell.java | 2 +-
.../org/apache/cassandra/fql/FullQueryLogger.java | 2 +-
.../org/apache/cassandra/utils/ObjectSizes.java | 8 +-
.../unit/org/apache/cassandra/db/CellSpecTest.java | 121 +++++++++++++++++++++
9 files changed, 138 insertions(+), 8 deletions(-)
diff --git a/CHANGES.txt b/CHANGES.txt
index aef6fe1..f79cf30 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -2,6 +2,7 @@
* Fix clustering order logic in CREATE MATERIALIZED VIEW (CASSANDRA-16898)
4.0.1
+ * org.apache.cassandra.db.rows.ArrayCell#unsharedHeapSizeExcludingData
includes data twice (CASSANDRA-16900)
* Exclude Jackson 1.x transitive dependency of hadoop* provided dependencies
(CASSANDRA-16854)
* Tolerate missing DNS entry when completing a host replacement
(CASSANDRA-16873)
* Harden PrunableArrayQueue against Pruner implementations that might throw
exceptions (CASSANDRA-16866)
diff --git a/src/java/org/apache/cassandra/cql3/ColumnIdentifier.java
b/src/java/org/apache/cassandra/cql3/ColumnIdentifier.java
index e7bf7b9..889f23a 100644
--- a/src/java/org/apache/cassandra/cql3/ColumnIdentifier.java
+++ b/src/java/org/apache/cassandra/cql3/ColumnIdentifier.java
@@ -205,7 +205,7 @@ public class ColumnIdentifier implements IMeasurableMemory,
Comparable<ColumnIde
public long unsharedHeapSizeExcludingData()
{
return EMPTY_SIZE
- + ObjectSizes.sizeOnHeapExcludingData(bytes)
+ + ObjectSizes.sizeOfEmptyHeapByteBuffer()
+ ObjectSizes.sizeOf(text);
}
diff --git a/src/java/org/apache/cassandra/db/rows/ArrayCell.java
b/src/java/org/apache/cassandra/db/rows/ArrayCell.java
index edeb65c..c4fdd14 100644
--- a/src/java/org/apache/cassandra/db/rows/ArrayCell.java
+++ b/src/java/org/apache/cassandra/db/rows/ArrayCell.java
@@ -112,6 +112,6 @@ public class ArrayCell extends AbstractCell<byte[]>
public long unsharedHeapSizeExcludingData()
{
- return EMPTY_SIZE + ObjectSizes.sizeOfArray(value) + value.length +
(path == null ? 0 : path.unsharedHeapSizeExcludingData());
+ return EMPTY_SIZE + ObjectSizes.sizeOfEmptyByteArray() + (path == null
? 0 : path.unsharedHeapSizeExcludingData());
}
}
diff --git a/src/java/org/apache/cassandra/db/rows/BufferCell.java
b/src/java/org/apache/cassandra/db/rows/BufferCell.java
index 3795b0c..55fc4b4 100644
--- a/src/java/org/apache/cassandra/db/rows/BufferCell.java
+++ b/src/java/org/apache/cassandra/db/rows/BufferCell.java
@@ -28,6 +28,8 @@ import org.apache.cassandra.utils.ByteBufferUtil;
import org.apache.cassandra.utils.ObjectSizes;
import org.apache.cassandra.utils.memory.AbstractAllocator;
+import static java.lang.String.format;
+
public class BufferCell extends AbstractCell<ByteBuffer>
{
private static final long EMPTY_SIZE = ObjectSizes.measure(new
BufferCell(ColumnMetadata.regularColumn("", "", "", ByteType.instance), 0L, 0,
0, ByteBufferUtil.EMPTY_BYTE_BUFFER, null));
@@ -43,7 +45,7 @@ public class BufferCell extends AbstractCell<ByteBuffer>
{
super(column);
assert !column.isPrimaryKeyColumn();
- assert column.isComplex() == (path != null);
+ assert column.isComplex() == (path != null) : format("Column %s.%s(%s:
%s) isComplex: %b with cellpath: %s", column.ksName, column.cfName,
column.name, column.type.toString(), column.isComplex(), path);
this.timestamp = timestamp;
this.ttl = ttl;
this.localDeletionTime = localDeletionTime;
@@ -142,6 +144,6 @@ public class BufferCell extends AbstractCell<ByteBuffer>
public long unsharedHeapSizeExcludingData()
{
- return EMPTY_SIZE + ObjectSizes.sizeOnHeapExcludingData(value) + (path
== null ? 0 : path.unsharedHeapSizeExcludingData());
+ return EMPTY_SIZE + ObjectSizes.sizeOfEmptyHeapByteBuffer() + (path ==
null ? 0 : path.unsharedHeapSizeExcludingData());
}
}
diff --git a/src/java/org/apache/cassandra/db/rows/CellPath.java
b/src/java/org/apache/cassandra/db/rows/CellPath.java
index 1bf8b8f..50496a1 100644
--- a/src/java/org/apache/cassandra/db/rows/CellPath.java
+++ b/src/java/org/apache/cassandra/db/rows/CellPath.java
@@ -127,7 +127,7 @@ public abstract class CellPath
public long unsharedHeapSizeExcludingData()
{
- return EMPTY_SIZE + ObjectSizes.sizeOnHeapExcludingData(value);
+ return EMPTY_SIZE + ObjectSizes.sizeOfEmptyHeapByteBuffer();
}
}
diff --git a/src/java/org/apache/cassandra/db/rows/NativeCell.java
b/src/java/org/apache/cassandra/db/rows/NativeCell.java
index 52d9ab9..02e0008 100644
--- a/src/java/org/apache/cassandra/db/rows/NativeCell.java
+++ b/src/java/org/apache/cassandra/db/rows/NativeCell.java
@@ -78,7 +78,7 @@ public class NativeCell extends AbstractCell<ByteBuffer>
assert column.isComplex() == (path != null);
if (path != null)
{
- assert path.size() == 1;
+ assert path.size() == 1 : String.format("Expected path size to be
1 but was not; %s", path);
size += 4 + path.get(0).remaining();
}
diff --git a/src/java/org/apache/cassandra/fql/FullQueryLogger.java
b/src/java/org/apache/cassandra/fql/FullQueryLogger.java
index 4b40635..9725cea 100644
--- a/src/java/org/apache/cassandra/fql/FullQueryLogger.java
+++ b/src/java/org/apache/cassandra/fql/FullQueryLogger.java
@@ -80,7 +80,7 @@ public class FullQueryLogger implements QueryEvents.Listener
public static final String QUERIES = "queries";
public static final String VALUES = "values";
- private static final int EMPTY_BYTEBUFFER_SIZE =
Ints.checkedCast(ObjectSizes.sizeOnHeapExcludingData(ByteBuffer.allocate(0)));
+ private static final int EMPTY_BYTEBUFFER_SIZE =
Ints.checkedCast(ObjectSizes.sizeOfEmptyHeapByteBuffer());
private static final int EMPTY_LIST_SIZE =
Ints.checkedCast(ObjectSizes.measureDeep(new ArrayList(0)));
private static final int EMPTY_BYTEBUF_SIZE;
diff --git a/src/java/org/apache/cassandra/utils/ObjectSizes.java
b/src/java/org/apache/cassandra/utils/ObjectSizes.java
index fa02ba7..04e5c65 100644
--- a/src/java/org/apache/cassandra/utils/ObjectSizes.java
+++ b/src/java/org/apache/cassandra/utils/ObjectSizes.java
@@ -37,6 +37,7 @@ public class ObjectSizes
.ignoreKnownSingletons();
private static final long BUFFER_EMPTY_SIZE =
measure(ByteBufferUtil.EMPTY_BYTE_BUFFER);
+ private static final long BYTE_ARRAY_EMPTY_SIZE = measure(new byte[0]);
private static final long STRING_EMPTY_SIZE = measure("");
/**
@@ -128,11 +129,16 @@ public class ObjectSizes
return BUFFER_EMPTY_SIZE + sizeOfArray(buffer.capacity(), 1);
}
- public static long sizeOnHeapExcludingData(ByteBuffer buffer)
+ public static long sizeOfEmptyHeapByteBuffer()
{
return BUFFER_EMPTY_SIZE;
}
+ public static long sizeOfEmptyByteArray()
+ {
+ return BYTE_ARRAY_EMPTY_SIZE;
+ }
+
/**
* Memory a String consumes
* @param str String to calculate memory size of
diff --git a/test/unit/org/apache/cassandra/db/CellSpecTest.java
b/test/unit/org/apache/cassandra/db/CellSpecTest.java
new file mode 100644
index 0000000..44fc625
--- /dev/null
+++ b/test/unit/org/apache/cassandra/db/CellSpecTest.java
@@ -0,0 +1,121 @@
+/*
+ * 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.ArrayList;
+import java.util.Collection;
+import java.util.List;
+import java.util.concurrent.CompletableFuture;
+import java.util.function.BiConsumer;
+import java.util.stream.Collectors;
+
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+
+import org.apache.cassandra.db.marshal.BytesType;
+import org.apache.cassandra.db.marshal.ListType;
+import org.apache.cassandra.db.rows.ArrayCell;
+import org.apache.cassandra.db.rows.BufferCell;
+import org.apache.cassandra.db.rows.Cell;
+import org.apache.cassandra.db.rows.CellPath;
+import org.apache.cassandra.db.rows.NativeCell;
+import org.apache.cassandra.schema.ColumnMetadata;
+import org.apache.cassandra.schema.TableMetadata;
+import org.apache.cassandra.utils.ObjectSizes;
+import org.apache.cassandra.utils.UUIDGen;
+import org.apache.cassandra.utils.concurrent.OpOrder;
+import org.apache.cassandra.utils.memory.NativeAllocator;
+import org.apache.cassandra.utils.memory.NativePool;
+import org.assertj.core.api.Assertions;
+
+import static org.apache.cassandra.utils.ByteBufferUtil.bytes;
+
+@RunWith(Parameterized.class)
+public class CellSpecTest
+{
+ private final Cell<?> cell;
+
+ @SuppressWarnings("unused")
+ public CellSpecTest(String ignoreOnlyUsedForBetterTestName, Cell<?> cell)
+ {
+ this.cell = cell;
+ }
+
+ @Test
+ public void unsharedHeapSizeExcludingData()
+ {
+ long empty = ObjectSizes.measure(cell);
+ long expected;
+ if (cell instanceof NativeCell)
+ {
+ // NativeCell stores the contents off-heap, so the cost on-heap is
just the object's empty case
+ expected = empty;
+ }
+ else
+ {
+ // size should be: empty + valuePtr +
path.unsharedHeapSizeExcludingData() if present
+ expected = empty + valuePtrSize(cell.value());
+ if (cell.path() != null)
+ expected += cell.path().unsharedHeapSizeExcludingData();
+ }
+
+ Assertions.assertThat(cell.unsharedHeapSizeExcludingData())
+ .isEqualTo(expected);
+ }
+
+ private static long valuePtrSize(Object value)
+ {
+ if (value instanceof ByteBuffer)
+ return ObjectSizes.sizeOfEmptyHeapByteBuffer();
+ else if (value instanceof byte[])
+ return ObjectSizes.sizeOfEmptyByteArray();
+ throw new IllegalArgumentException("Unsupported type: " +
value.getClass());
+ }
+
+ @Parameterized.Parameters(name = "{0}")
+ public static Collection<Object[]> data() {
+ TableMetadata table = TableMetadata.builder("testing", "testing")
+ .addPartitionKeyColumn("pk",
BytesType.instance)
+ .build();
+
+ byte[] rawBytes = { 0, 1, 2, 3, 4, 5, 6 };
+ ByteBuffer bbBytes = ByteBuffer.wrap(rawBytes);
+ NativePool pool = new NativePool(1024, 1024, 1, () ->
CompletableFuture.completedFuture(true));
+ NativeAllocator allocator = pool.newAllocator();
+ OpOrder order = new OpOrder();
+
+ List<Cell<?>> tests = new ArrayList<>();
+ BiConsumer<ColumnMetadata, CellPath> fn = (column, path) -> {
+ tests.add(new ArrayCell(column, 1234, 1, 1, rawBytes, path));
+ tests.add(new BufferCell(column, 1234, 1, 1, bbBytes, path));
+ tests.add(new NativeCell(allocator, order.getCurrent(), column,
1234, 1, 1, bbBytes, path));
+ };
+ // simple
+ fn.accept(ColumnMetadata.regularColumn(table, bytes("simple"),
BytesType.instance), null);
+
+ // complex
+ // seems NativeCell does not allow CellPath.TOP, or CellPath.BOTTOM
+ fn.accept(ColumnMetadata.regularColumn(table, bytes("complex"),
ListType.getInstance(BytesType.instance, true)),
CellPath.create(bytes(UUIDGen.getTimeUUID())));
+
+ return tests.stream().map(a -> new Object[]
{a.getClass().getSimpleName() + ":" + (a.path() == null ? "simple" :
"complex"), a}).collect(Collectors.toList());
+ }
+
+}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]