koo created CASSANDRA-21587:
-------------------------------
Summary: 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
Reporter: koo
=== DESCRIPTION ===
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(i)}} 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(i)}} 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]
