natas_hw
/
cassandra
mirror of https://github.com/apache/cassandra
0
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

[CASSANDRA-21536] Profile pollution in AbstractType.writeValue makes serialization slow for all column types 

Description:
AbstractType.writeValue() is one shared method. All column types use it.

Inside writeValue(), it calls valueLengthIfFixed(). This is a virtual call. Many types override this method (Int32Type, LongType, UTF8Type, ...).

In a real cluster, many column types pass through writeValue(). So the profile always sees lots types.

Because of this, the JIT cannot inline the call. It stays as a vtable call. Also, the compiled body of writeValue() becomes big, so the JIT refuses to inline writeValue() itself ("already compiled into a big method").

We also see the same itable/vtable stubs in async-profiler output from a real cluster, running with default production options and a normal workload.

How to solve:
Add a final int field to AbstractType. Set it in the constructor. writeValue() reads this field instead of calling valueLengthIfFixed().

A field read needs no type profile. So profile pollution has no effect on it. The valueLengthIfFixed() method is not changed. Only the write path uses the field.

Result:
Production is always the polluted state, so this is the real-world comparison

 JMH, JDK 17 / arm64, 1 thread, 10 x 1s warmup and measurement:

                    before          after       improvement
   readValue        49.99M ops/s    56.44M ops/s  +12.90%
   writeValue       99.63M ops/s   114.46M ops/s  +14.88%
This commit is contained in:
koo.taejin 2026-07-27 23:48:03 +09:00
parent af202bddc2
commit ea86293425
20 changed files with 150 additions and 105 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
CHANGES.txt
View File

@ -1,4 +1,5 @@
7.0
* Avoid megamorphic calls when serializing and deserializing fixed-length values (CASSANDRA-21536)
* Don't increment client metrics on messaging service connection unpause (CASSANDRA-21491)
* Add nodetool getreplicas (CASSANDRA-17665)
* Implementation of CEP-49: Hardware-accelerated compression (CASSANDRA-20975)
@ -8663,4 +8664,3 @@ Full list of issues resolved in 0.4 is at https://issues.apache.org/jira/secure/
* Added FlushPeriodInMinutes configuration parameter to force
flushing of infrequently-updated ColumnFamilies

8
src/java/org/apache/cassandra/db/marshal/AbstractTimeUUIDType.java
View File

@ -39,7 +39,7 @@ public abstract class AbstractTimeUUIDType<T> extends TemporalType<T>
{
AbstractTimeUUIDType()
{
super(ComparisonType.CUSTOM);
super(ComparisonType.CUSTOM, 16);
} // singleton
@Override
@ -193,12 +193,6 @@ public abstract class AbstractTimeUUIDType<T> extends TemporalType<T>
return super.decomposeUntyped(value);
}
@Override
public int valueLengthIfFixed()
{
return 16;
}
@Override
public long toTimeInMillis(ByteBuffer value)
{

29
src/java/org/apache/cassandra/db/marshal/AbstractType.java
View File

@ -90,11 +90,18 @@ public abstract class AbstractType<T> implements Comparator<ByteBuffer>, Assignm
public final ComparisonType comparisonType;
public final boolean isByteOrderComparable;
public final ValueComparators comparatorSet;
private final int valueLengthIfFixed;
protected AbstractType(ComparisonType comparisonType)
{
this(comparisonType, VARIABLE_LENGTH);
}
protected AbstractType(ComparisonType comparisonType, int valueLengthIfFixed)
{
this.comparisonType = comparisonType;
this.isByteOrderComparable = comparisonType == ComparisonType.BYTE_ORDER;
this.valueLengthIfFixed = valueLengthIfFixed;
reverseComparator = (o1, o2) -> AbstractType.this.compare(o2, o1);
try
{
@ -384,12 +391,12 @@ public abstract class AbstractType<T> implements Comparator<ByteBuffer>, Assignm
/**
* Similar to {@link #isValueCompatibleWith(AbstractType)}, but takes into account {@link Cell} encoding.
* In particular, this method doesn't consider two types serialization compatible if one of them has fixed
* length (overrides {@link #valueLengthIfFixed()}, and the other one doesn't.
* length, and the other one doesn't.
*/
public boolean isSerializationCompatibleWith(AbstractType<?> previous)
{
return isValueCompatibleWith(previous)
&& valueLengthIfFixed() == previous.valueLengthIfFixed()
&& valueLengthIfFixed == previous.valueLengthIfFixed
&& isMultiCell() == previous.isMultiCell();
}
@ -496,9 +503,9 @@ public abstract class AbstractType<T> implements Comparator<ByteBuffer>, Assignm
* <li> see {@link #skipValue} </li>
* </lu>
*/
public int valueLengthIfFixed()
public final int valueLengthIfFixed()
{
return VARIABLE_LENGTH;
return valueLengthIfFixed;
}
/**
@ -508,7 +515,7 @@ public abstract class AbstractType<T> implements Comparator<ByteBuffer>, Assignm
*/
public final boolean isValueLengthFixed()
{
return valueLengthIfFixed() != VARIABLE_LENGTH;
return valueLengthIfFixed != VARIABLE_LENGTH;
}
/**
@ -570,7 +577,7 @@ public abstract class AbstractType<T> implements Comparator<ByteBuffer>, Assignm
public <V> void writeValue(V value, ValueAccessor<V> accessor, DataOutputPlus out) throws IOException
{
assert !isNull(value, accessor) : "bytes should not be null for type " + this;
int expectedValueLength = valueLengthIfFixed();
int expectedValueLength = valueLengthIfFixed;
if (expectedValueLength >= 0)
{
int actualValueLength = accessor.size(value);
@ -589,7 +596,7 @@ public abstract class AbstractType<T> implements Comparator<ByteBuffer>, Assignm
public <V> void writeValue(IndexedValueHolder<V> valueHolder, int i, ValueAccessor<V> accessor, DataOutputPlus out) throws IOException
{
assert !valueHolder.isNull(i) : "bytes should not be null for type " + this;
int expectedValueLength = valueLengthIfFixed();
int expectedValueLength = valueLengthIfFixed;
if (expectedValueLength >= 0)
{
int actualValueLength = valueHolder.size(i);
@ -613,7 +620,7 @@ public abstract class AbstractType<T> implements Comparator<ByteBuffer>, Assignm
public <V> long writtenLength(V value, ValueAccessor<V> accessor)
{
assert !accessor.isEmpty(value) : "bytes should not be empty for type " + this;
return valueLengthIfFixed() >= 0
return valueLengthIfFixed >= 0
? accessor.size(value) // if the size is wrong, this will be detected in writeValue
: accessor.sizeWithVIntLength(value);
}
@ -621,7 +628,7 @@ public abstract class AbstractType<T> implements Comparator<ByteBuffer>, Assignm
public <V> long writtenLength(IndexedValueHolder<V> valueHolder, int i, ValueAccessor<V> accessor)
{
assert !valueHolder.isNull(i) : "bytes should not be null for type " + this;
return valueLengthIfFixed() >= 0
return valueLengthIfFixed >= 0
? valueHolder.size(i) // if the size is wrong, this will be detected in writeValue
: accessor.sizeWithVIntLength(valueHolder, i);
}
@ -643,7 +650,7 @@ public abstract class AbstractType<T> implements Comparator<ByteBuffer>, Assignm
public <V> V read(ValueAccessor<V> accessor, DataInputPlus in, int maxValueSize) throws IOException
{
int length = valueLengthIfFixed();
int length = valueLengthIfFixed;
if (length >= 0)
return accessor.read(in, length);
@ -664,7 +671,7 @@ public abstract class AbstractType<T> implements Comparator<ByteBuffer>, Assignm
public void skipValue(DataInputPlus in) throws IOException
{
int length = valueLengthIfFixed();
int length = valueLengthIfFixed;
if (length >= 0)
in.skipBytesFully(length);
else

8
src/java/org/apache/cassandra/db/marshal/BooleanType.java
View File

@ -36,7 +36,7 @@ public class BooleanType extends AbstractType<Boolean>
private static final ArgumentDeserializer ARGUMENT_DESERIALIZER = new DefaultArgumentDeserializer(instance);
private static final ByteBuffer MASKED_VALUE = instance.decompose(false);
BooleanType() {super(ComparisonType.CUSTOM);} // singleton
BooleanType() {super(ComparisonType.CUSTOM, 1);} // singleton
@Override
public boolean allowsEmpty()
@ -127,12 +127,6 @@ public class BooleanType extends AbstractType<Boolean>
return ARGUMENT_DESERIALIZER;
}
@Override
public int valueLengthIfFixed()
{
return 1;
}
@Override
public ByteBuffer getMaskedValue()
{

8
src/java/org/apache/cassandra/db/marshal/DateType.java
View File

@ -49,7 +49,7 @@ public class DateType extends AbstractType<Date>
private static final ArgumentDeserializer ARGUMENT_DESERIALIZER = new DefaultArgumentDeserializer(instance);
private static final ByteBuffer MASKED_VALUE = instance.decompose(new Date(0));
DateType() {super(ComparisonType.BYTE_ORDER);} // singleton
DateType() {super(ComparisonType.BYTE_ORDER, 8);} // singleton
public boolean isEmptyValueMeaningless()
{
@ -143,12 +143,6 @@ public class DateType extends AbstractType<Date>
return ARGUMENT_DESERIALIZER;
}
@Override
public int valueLengthIfFixed()
{
return 8;
}
@Override
public ByteBuffer getMaskedValue()
{

8
src/java/org/apache/cassandra/db/marshal/DoubleType.java
View File

@ -40,7 +40,7 @@ public class DoubleType extends NumberType<Double>
private static final ByteBuffer MASKED_VALUE = instance.decompose(0d);
DoubleType() {super(ComparisonType.CUSTOM);} // singleton
DoubleType() {super(ComparisonType.CUSTOM, 8);} // singleton
@Override
public boolean allowsEmpty()
@ -145,12 +145,6 @@ public class DoubleType extends NumberType<Double>
};
}
@Override
public int valueLengthIfFixed()
{
return 8;
}
@Override
public ByteBuffer add(Number left, Number right)
{

8
src/java/org/apache/cassandra/db/marshal/EmptyType.java
View File

@ -71,7 +71,7 @@ public class EmptyType extends AbstractType<Void>
public static final EmptyType instance = new EmptyType();
private EmptyType() {super(ComparisonType.CUSTOM);} // singleton
private EmptyType() {super(ComparisonType.CUSTOM, 0);} // singleton
@Override
public <V> ByteSource asComparableBytes(ValueAccessor<V> accessor, V data, ByteComparable.Version version)
@ -137,12 +137,6 @@ public class EmptyType extends AbstractType<Void>
throw new UnsupportedOperationException();
}
@Override
public int valueLengthIfFixed()
{
return 0;
}
@Override
public <V> long writtenLength(V value, ValueAccessor<V> accessor)
{

8
src/java/org/apache/cassandra/db/marshal/FloatType.java
View File

@ -41,7 +41,7 @@ public class FloatType extends NumberType<Float>
private static final ByteBuffer MASKED_VALUE = instance.decompose(0f);
FloatType() {super(ComparisonType.CUSTOM);} // singleton
FloatType() {super(ComparisonType.CUSTOM, 4);} // singleton
@Override
public boolean allowsEmpty()
@ -146,12 +146,6 @@ public class FloatType extends NumberType<Float>
};
}
@Override
public int valueLengthIfFixed()
{
return 4;
}
@Override
public ByteBuffer add(Number left, Number right)
{

8
src/java/org/apache/cassandra/db/marshal/Int32Type.java
View File

@ -43,7 +43,7 @@ public class Int32Type extends NumberType<Integer>
Int32Type()
{
super(ComparisonType.CUSTOM);
super(ComparisonType.CUSTOM, 4);
} // singleton
@Override
@ -152,12 +152,6 @@ public class Int32Type extends NumberType<Integer>
};
}
@Override
public int valueLengthIfFixed()
{
return 4;
}
@Override
public ByteBuffer add(Number left, Number right)
{

8
src/java/org/apache/cassandra/db/marshal/LexicalUUIDType.java
View File

@ -44,7 +44,7 @@ public class LexicalUUIDType extends AbstractType<UUID>
LexicalUUIDType()
{
super(ComparisonType.CUSTOM);
super(ComparisonType.CUSTOM, 16);
} // singleton
@Override
@ -148,12 +148,6 @@ public class LexicalUUIDType extends AbstractType<UUID>
return ARGUMENT_DESERIALIZER;
}
@Override
public int valueLengthIfFixed()
{
return 16;
}
@Override
public ByteBuffer getMaskedValue()
{

8
src/java/org/apache/cassandra/db/marshal/LongType.java
View File

@ -41,7 +41,7 @@ public class LongType extends NumberType<Long>
private static final ByteBuffer MASKED_VALUE = instance.decompose(0L);
LongType() {super(ComparisonType.CUSTOM);} // singleton
LongType() {super(ComparisonType.CUSTOM, 8);} // singleton
@Override
public boolean allowsEmpty()
@ -170,12 +170,6 @@ public class LongType extends NumberType<Long>
};
}
@Override
public int valueLengthIfFixed()
{
return 8;
}
@Override
public ByteBuffer add(Number left, Number right)
{

6
src/java/org/apache/cassandra/db/marshal/MultiElementType.java
View File

@ -38,6 +38,11 @@ public abstract class MultiElementType<T> extends AbstractType<T>
super(comparisonType);
}
protected MultiElementType(ComparisonType comparisonType, int valueLengthIfFixed)
{
super(comparisonType, valueLengthIfFixed);
}
/**
* Returns the serialized representation of the value composed of the specified elements.
*
@ -133,4 +138,3 @@ public abstract class MultiElementType<T> extends AbstractType<T>
throw new UnsupportedOperationException(this + " does not support retrieving elements by key or index");
}
}

5
src/java/org/apache/cassandra/db/marshal/NumberType.java
View File

@ -34,6 +34,11 @@ public abstract class NumberType<T extends Number> extends AbstractType<T>
super(comparisonType);
}
protected NumberType(ComparisonType comparisonType, int valueLengthIfFixed)
{
super(comparisonType, valueLengthIfFixed);
}
/**
* Checks if this type support floating point numbers.
* @return {@code true} if this type support floating point numbers, {@code false} otherwise.

8
src/java/org/apache/cassandra/db/marshal/ReversedType.java
View File

@ -57,7 +57,7 @@ public class ReversedType<T> extends AbstractType<T>
private ReversedType(AbstractType<T> baseType)
{
super(ComparisonType.CUSTOM);
super(ComparisonType.CUSTOM, baseType.valueLengthIfFixed());
this.baseType = baseType;
}
@ -181,12 +181,6 @@ public class ReversedType<T> extends AbstractType<T>
return getInstance(baseType.withUpdatedUserType(udt));
}
@Override
public int valueLengthIfFixed()
{
return baseType.valueLengthIfFixed();
}
@Override
public boolean isReversed()
{

5
src/java/org/apache/cassandra/db/marshal/TemporalType.java
View File

@ -37,6 +37,11 @@ public abstract class TemporalType<T> extends AbstractType<T>
super(comparisonType);
}
protected TemporalType(ComparisonType comparisonType, int valueLengthIfFixed)
{
super(comparisonType, valueLengthIfFixed);
}
/**
* Returns the current temporal value.
* @return the current temporal value.

8
src/java/org/apache/cassandra/db/marshal/TimestampType.java
View File

@ -53,7 +53,7 @@ public class TimestampType extends TemporalType<Date>
private static final ByteBuffer MASKED_VALUE = instance.decompose(new Date(0));
private TimestampType() {super(ComparisonType.CUSTOM);} // singleton
private TimestampType() {super(ComparisonType.CUSTOM, 8);} // singleton
@Override
public boolean allowsEmpty()
@ -166,12 +166,6 @@ public class TimestampType extends TemporalType<Date>
return TimestampSerializer.instance;
}
@Override
public int valueLengthIfFixed()
{
return 8;
}
@Override
protected void validateDuration(Duration duration)
{

8
src/java/org/apache/cassandra/db/marshal/UUIDType.java
View File

@ -56,7 +56,7 @@ public class UUIDType extends AbstractType<UUID>
UUIDType()
{
super(ComparisonType.CUSTOM);
super(ComparisonType.CUSTOM, 16);
}
@Override
@ -252,12 +252,6 @@ public class UUIDType extends AbstractType<UUID>
return (uuid.get(6) & 0xf0) >> 4;
}
@Override
public int valueLengthIfFixed()
{
return 16;
}
@Override
public ByteBuffer getMaskedValue()
{

12
src/java/org/apache/cassandra/db/marshal/VectorType.java
View File

@ -82,20 +82,16 @@ public final class VectorType<T> extends MultiElementType<List<T>>
public final AbstractType<T> elementType;
public final int dimension;
private final TypeSerializer<T> elementSerializer;
private final int valueLengthIfFixed;
private final VectorSerializer serializer;
private VectorType(AbstractType<T> elementType, int dimension)
{
super(ComparisonType.CUSTOM);
super(ComparisonType.CUSTOM, valueLengthIfFixed(elementType, dimension));
if (dimension <= 0)
throw new InvalidRequestException(String.format("vectors may only have positive dimensions; given %d", dimension));
this.elementType = elementType;
this.dimension = dimension;
this.elementSerializer = elementType.getSerializer();
this.valueLengthIfFixed = elementType.isValueLengthFixed() ?
elementType.valueLengthIfFixed() * dimension :
super.valueLengthIfFixed();
this.serializer = elementType.isValueLengthFixed() ?
new FixedLengthSerializer() :
new VariableLengthSerializer();
@ -126,10 +122,10 @@ public final class VectorType<T> extends MultiElementType<List<T>>
return getSerializer().compareCustom(left, accessorL, right, accessorR);
}
@Override
public int valueLengthIfFixed()
private static int valueLengthIfFixed(AbstractType<?> elementType, int dimension)
{
return valueLengthIfFixed;
int elementLength = elementType.valueLengthIfFixed();
return elementLength >= 0 ? elementLength * dimension : elementLength;
}
@Override

94
test/microbench/org/apache/cassandra/test/microbench/AbstractTypeSerializationBench.java Normal file
View File

@ -0,0 +1,94 @@
/*
* 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.test.microbench;
import java.io.IOException;
import java.nio.ByteBuffer;
import java.util.concurrent.TimeUnit;
import org.openjdk.jmh.annotations.Benchmark;
import org.openjdk.jmh.annotations.BenchmarkMode;
import org.openjdk.jmh.annotations.Fork;
import org.openjdk.jmh.annotations.Measurement;
import org.openjdk.jmh.annotations.Mode;
import org.openjdk.jmh.annotations.OutputTimeUnit;
import org.openjdk.jmh.annotations.Scope;
import org.openjdk.jmh.annotations.Setup;
import org.openjdk.jmh.annotations.State;
import org.openjdk.jmh.annotations.Threads;
import org.openjdk.jmh.annotations.Warmup;
import org.apache.cassandra.db.marshal.AbstractType;
import org.apache.cassandra.db.marshal.BooleanType;
import org.apache.cassandra.db.marshal.Int32Type;
import org.apache.cassandra.db.marshal.LongType;
import org.apache.cassandra.db.marshal.UUIDType;
import org.apache.cassandra.db.marshal.UTF8Type;
import org.apache.cassandra.io.util.DataInputBuffer;
import org.apache.cassandra.io.util.DataOutputBuffer;
import org.apache.cassandra.utils.ByteBufferUtil;
/**
* Measures value serialization when one call site sees the fixed- and variable-width types commonly used by a table.
*/
@BenchmarkMode(Mode.Throughput)
@OutputTimeUnit(TimeUnit.SECONDS)
@Warmup(iterations = 5, time = 1)
@Measurement(iterations = 5, time = 1)
@Fork(value = 1, jvmArgsAppend = { "-Xmx1G", "-Djmh.executor=CUSTOM", "-Djmh.executor.class=org.apache.cassandra.test.microbench.FastThreadExecutor" })
@Threads(1)
@State(Scope.Thread)
public class AbstractTypeSerializationBench
{
private final AbstractType<?>[] types = { Int32Type.instance, LongType.instance, BooleanType.instance, UUIDType.instance,
UTF8Type.instance, Int32Type.instance, LongType.instance, UTF8Type.instance };
private final ByteBuffer[] values = { ByteBufferUtil.bytes(42), ByteBufferUtil.bytes(42L), ByteBuffer.wrap(new byte[] { 1 }),
ByteBuffer.wrap(new byte[16]), ByteBufferUtil.bytes("cassandra"), ByteBufferUtil.bytes(42),
ByteBufferUtil.bytes(42L), ByteBufferUtil.bytes("cassandra") };
private final ByteBuffer[] serializedValues = new ByteBuffer[types.length];
private final DataOutputBuffer out = new DataOutputBuffer();
private int index;
@Setup
public void setup() throws IOException
{
for (int i = 0; i < types.length; i++)
{
out.clear();
types[i].writeValue(values[i], out);
serializedValues[i] = out.asNewBuffer();
}
}
@Benchmark
public int writeValue() throws IOException
{
int i = index++ & (types.length - 1);
out.clear();
types[i].writeValue(values[i], out);
return out.getLength();
}
@Benchmark
public ByteBuffer readValue() throws IOException
{
int i = index++ & (types.length - 1);
return types[i].readBuffer(new DataInputBuffer(serializedValues[i], true));
}
}

6
test/unit/org/apache/cassandra/db/marshal/AbstractTypeTest.java
View File

@ -739,6 +739,12 @@ public class AbstractTypeTest
}});
}
@Test
public void valueLengthIfFixedIsFinal() throws NoSuchMethodException
{
assertThat(Modifier.isFinal(AbstractType.class.getMethod("valueLengthIfFixed").getModifiers())).isTrue();
}
@Test
@SuppressWarnings({"rawtypes", "unchecked"})
public void serde()