[
https://issues.apache.org/jira/browse/CASSANDRA-21587?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18108086#comment-18108086
]
koo commented on CASSANDRA-21587:
---------------------------------
This is the code I benchmarked.
It showed these results in my environment, but they may vary depending on the
environment. I think it could also depend on how much of the total time is
spent in the deserializeOne method.
Thank you for checking. :)
{code:java}
package org.apache.cassandra.test.microbench;
import java.io.IOException;
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.Param;
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.config.DatabaseDescriptor;
import org.apache.cassandra.db.ClusteringBoundOrBoundary;
import org.apache.cassandra.db.ClusteringComparator;
import org.apache.cassandra.db.ClusteringPrefix;
import org.apache.cassandra.db.SerializationHeader;
import org.apache.cassandra.db.marshal.Int32Type;
import org.apache.cassandra.db.rows.UnfilteredSerializer;
import org.apache.cassandra.io.sstable.format.SSTableFormat;
import org.apache.cassandra.io.util.DataInputBuffer;
import org.apache.cassandra.io.util.DataOutputBuffer;
import org.apache.cassandra.schema.TableMetadata;
import org.apache.cassandra.utils.ByteBufferUtil;
/**
* Measures {@link ClusteringPrefix.Deserializer} with the clustering-type list
implementations produced by the
* memtable and SSTable paths. Before CASSANDRA-21587, adding the SSTable path
introduces {@code ArrayList}; after
* the fix, it shares the multi-column {@code ImmutableList} implementation
used by the memtable path. Every
* benchmark operation reads one non-empty component so that the header sources
perform the same work.
*/
@BenchmarkMode(Mode.AverageTime)
@OutputTimeUnit(TimeUnit.NANOSECONDS)
@Warmup(iterations = 5, time = 1)
@Measurement(iterations = 5, time = 1)
@Fork(value = 3,
jvmArgsAppend = { "-Xmx1G", "-Djmh.executor=CUSTOM",
"-Djmh.executor.class=org.apache.cassandra.test.microbench.FastThreadExecutor"
})
@Threads(1)
@State(Scope.Thread)
public class ClusteringPrefixDeserializerBench
{
@Param({ "1", "2", "3" })
public int headerSources;
private DeserializerState[] states;
private int index;
@Setup
public void setup() throws Exception
{
DatabaseDescriptor.daemonInitialization();
TableMetadata oneColumn = schema("one_column", 1);
TableMetadata twoColumns = schema("two_columns", 2);
SerializationHeader oneColumnHeader =
SerializationHeader.makeWithoutStats(oneColumn);
SerializationHeader twoColumnHeader =
SerializationHeader.makeWithoutStats(twoColumns);
SerializationHeader sstableHeader = deserializeHeader(twoColumnHeader,
twoColumns);
DeserializerState regularImmutable = state(twoColumns.comparator,
twoColumnHeader);
DeserializerState singletonImmutable = state(oneColumn.comparator,
oneColumnHeader);
DeserializerState sstable = state(twoColumns.comparator, sstableHeader);
switch (headerSources)
{
case 1:
states = new DeserializerState[] { regularImmutable };
break;
case 2:
states = new DeserializerState[] { regularImmutable,
singletonImmutable };
break;
case 3:
states = new DeserializerState[] { regularImmutable,
singletonImmutable, sstable };
break;
default:
throw new AssertionError("Unsupported number of header sources:
" + headerSources);
}
}
@Benchmark
public ClusteringBoundOrBoundary<byte[]> deserializeBound() throws
IOException
{
DeserializerState state = states[index++ % states.length];
state.in.buffer().rewind();
state.deserializer.prepare(UnfilteredSerializer.IS_MARKER, 0);
return state.deserializer.deserializeNextBound();
}
private static TableMetadata schema(String name, int clusteringColumns)
{
TableMetadata.Builder builder =
TableMetadata.builder("ClusteringPrefixDeserializerBench", name)
.addPartitionKeyColumn("k", Int32Type.instance);
for (int i = 0; i < clusteringColumns; i++)
builder.addClusteringColumn("c" + i, Int32Type.instance);
return builder.build();
}
private static SerializationHeader deserializeHeader(SerializationHeader
header, TableMetadata metadata)
throws Exception
{
SSTableFormat<?, ?> format =
DatabaseDescriptor.getSelectedSSTableFormat();
try (DataOutputBuffer out = new DataOutputBuffer())
{
SerializationHeader.serializer.serialize(format.getLatestVersion(),
header.toComponent(), out);
SerializationHeader.Component component =
SerializationHeader.serializer.deserialize(format.getLatestVersion(),
new DataInputBuffer(out.buffer(), true));
return component.toHeader(metadata);
}
}
private static DeserializerState state(ClusteringComparator comparator,
SerializationHeader header)
throws IOException
{
try (DataOutputBuffer out = new DataOutputBuffer())
{
out.writeByte(ClusteringPrefix.Kind.INCL_START_BOUND.ordinal());
out.writeUnsignedShort(1);
out.writeUnsignedVInt(0);
header.clusteringTypes().get(0).writeValue(ByteBufferUtil.bytes(0),
out);
DataInputBuffer in = new DataInputBuffer(out.buffer(), true);
return new DeserializerState(in, new
ClusteringPrefix.Deserializer(comparator, in, header));
}
}
private static class DeserializerState
{
final DataInputBuffer in;
final ClusteringPrefix.Deserializer deserializer;
private DeserializerState(DataInputBuffer in,
ClusteringPrefix.Deserializer deserializer)
{
this.in = in;
this.deserializer = deserializer;
}
}
}
{code}
> Megamorphic call in ClusteringPrefix.Deserializer.deserializeOne slows down
> reads
> ---------------------------------------------------------------------------------
>
> Key: CASSANDRA-21587
> URL: https://issues.apache.org/jira/browse/CASSANDRA-21587
> Project: Apache Cassandra
> Issue Type: Improvement
> Components: Local/Other
> Reporter: koo
> Assignee: koo
> Priority: Normal
> Fix For: 6.x, 7.x
>
> Attachments: megamorphic_call_profile_example.png
>
> Time Spent: 10m
> Remaining Estimate: 0h
>
> h2. Problem
> {{ClusteringPrefix.Deserializer.deserializeOne()}} reads one clustering
> component at a
> time. It is the inner loop of a partition scan, so it runs very often. It has
> this line
> (ClusteringPrefix.java:674 on cassandra-5.0, :725 on trunk):
> {code:java}
> serializationHeader.clusteringTypes().get(i).readArray(in,
> DatabaseDescriptor.getMaxValueSize())
> {code}
> {{clusteringTypes()}} returns a {{{}java.util.List{}}}, so {{get}} is an
> interface call.
> Three classes can arrive here:
> ||Where the header comes from||Class of the list||
> |sstable Stats component read from disk|{{java.util.ArrayList}}|
> |flush or compaction, 1 clustering column|{{SingletonImmutableList}}|
> |flush or compaction, 2 or more clustering columns|{{RegularImmutableList}}|
> The first comes from {{AbstractTypeSerializer.deserializeList()}}
> ({{{}new ArrayList<>(size){}}}). The other two come from
> {{ClusteringComparator.subtypes()}}
> ({{{}ImmutableList.copyOf(...){}}}), which returns a different class for size
> 1 and size 2+.
> HotSpot can inline one or two receiver classes. With three it gives up and
> uses an itable
> stub, which costs much more than the work itself. {{get}} is only one array
> load.
> {{ClusteringComparator}} already has the comment
> {{{}// copy the list to ensure despatch is monomorphic{}}}, but it does not
> hold today.
> h3. Numbers
> Production node, async-profiler, 120s, 97,036 CPU samples:
> |{{deserializeOne}} (inclusive)|4,869|5.0% of all CPU|
> |itable stub from that one line|608|0.63% of all CPU|
> About {*}1/8 of the CPU spent in {{deserializeOne}} is only dispatch{*}, not
> real work.
> JMH benchmark with the receiver count controlled:
> |3 classes|1129.1 +- 17.1 ns/op|
> |2 classes|981.4 +- 34.8 ns/op|
> |1 class|934.7 +- 27.4 ns/op|
> Three to two is {*}13.1% faster{*}, and the intervals do not overlap.
> h2. Fix
> *Option 1 - one file, four lines.* Make
> {{AbstractTypeSerializer.deserializeList()}} return
> an immutable list. The return type stays {{{}List{}}}, so nothing else
> changes.
> {code:java}
> - List<AbstractType<?>> types = new ArrayList<>(size);
> + ImmutableList.Builder<AbstractType<?>> types =
> ImmutableList.builderWithExpectedSize(size);
> for (int i = 0; i < size; i++)
> types.add(deserialize(in));
> - return types;
> + return types.build();
> {code}
> *Option 2 - type the chain as {{ImmutableList}}*
> ({{{}ClusteringComparator{}}},
> {{{}SerializationHeader{}}}, the clustering serializers, {{{}Slice{}}},
> {{{}Slices{}}},
> {{{}IndexInfo{}}}, {{{}StatsMetadata{}}}; about 12 files, all mechanical).
> It could stop the same mistake from coming back.
> We suggest Option 1 first, and Option 2 as a follow-up if people want the
> type safety.
> h2. Notes
> * We see this on a cluster we run in production on 5.0.8, not only in the
> profile.
> * The same code is on trunk; only line numbers differ.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
