[
https://issues.apache.org/jira/browse/CASSANDRA-21587?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18115236#comment-18115236
]
Francisco Guerrero commented on CASSANDRA-21587:
------------------------------------------------
Hi [~taejin], thanks for the patch. I went with a more invasive approach
inspired by your optimization. I have a patch here:
https://github.com/frankgh/cassandra/commit/b706e120720f4c2a9553193dd86e57c5fe258b2b
, where I borrowed your benchmark.
Here's the summary of results comparing your approach versus the one I'm
proposing:
{noformat}
deserializeBound(headerSources=1)
------------------------------------------------------------
base 19.397 n=250 forks=50 sd= 1.645 (8.5%) 99.9% CI ± 0.346
opt12 19.543 n=250 forks=50 sd= 1.819 (9.3%) 99.9% CI ± 0.383
delta 0.146 ns/op (+0.75%)
unpaired Welch p= 0.346 (df=493.1) Mann-Whitney p= 0.300
resolvable effect (80% power) ≥ 0.434 ns/op (2.24% of base)
paired mean diff 0.146 ns/op 99.9% CI ± 1.151 p= 0.658 (23/50 rounds
favour opt12)
alloc/op base 72.000 opt12 72.000 -> +0.000 B/op (+0.00%) (varies
within variant: ±0.000/±0.000)
verdict no measurable difference
deserializeBound(headerSources=2)
------------------------------------------------------------
base 22.900 n=250 forks=50 sd= 1.645 (7.2%) 99.9% CI ± 0.347
opt12 21.957 n=250 forks=50 sd= 1.362 (6.2%) 99.9% CI ± 0.287
delta -0.943 ns/op (-4.12%)
unpaired Welch p=<0.001 (df=481.1) Mann-Whitney p=<0.001
resolvable effect (80% power) ≥ 0.378 ns/op (1.65% of base)
paired mean diff -0.943 ns/op 99.9% CI ± 0.899 p=<0.001 (34/50 rounds
favour opt12)
alloc/op base 72.000 opt12 72.000 -> -0.000 B/op (-0.00%) (varies
within variant: ±0.000/±0.000)
verdict opt12 faster by 4.12%
deserializeBound(headerSources=3)
------------------------------------------------------------
base 23.784 n=250 forks=50 sd= 1.635 (6.9%) 99.9% CI ± 0.344
opt12 21.533 n=250 forks=50 sd= 1.193 (5.5%) 99.9% CI ± 0.251
delta -2.252 ns/op (-9.47%)
unpaired Welch p=<0.001 (df=455.7) Mann-Whitney p=<0.001
resolvable effect (80% power) ≥ 0.359 ns/op (1.51% of base)
paired mean diff -2.252 ns/op 99.9% CI ± 0.868 p=<0.001 (42/50 rounds
favour opt12)
alloc/op base 72.000 opt12 72.000 -> -0.000 B/op (-0.00%) (varies
within variant: ±0.000/±0.000)
verdict opt12 faster by 9.47%
Summary (alpha=0.050)
deserializeBound(headerSources=1) no measurable difference p= 0.346
deserializeBound(headerSources=2) opt12 faster by 4.12% p=<0.001
deserializeBound(headerSources=3) opt12 faster by 9.47% p=<0.001
{noformat}
Let me know your thoughts on this alternate approach.
> 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: 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]
