diff --git a/CHANGES.txt b/CHANGES.txt index 2e77d2e23b..7f4b655830 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,4 +1,5 @@ 3.11.3 + * Remove BTree.Builder Recycler to reduce memory usage (CASSANDRA-13929) * Reduce nodetool GC thread count (CASSANDRA-14475) * Fix New SASI view creation during Index Redistribution (CASSANDRA-14055) * Remove string formatting lines from BufferPool hot path (CASSANDRA-14416) diff --git a/build.xml b/build.xml index 4edfbb1440..54c53720bf 100644 --- a/build.xml +++ b/build.xml @@ -422,8 +422,8 @@ - - + + diff --git a/src/java/org/apache/cassandra/db/columniterator/SSTableReversedIterator.java b/src/java/org/apache/cassandra/db/columniterator/SSTableReversedIterator.java index cf8798dbd5..6a0b7be628 100644 --- a/src/java/org/apache/cassandra/db/columniterator/SSTableReversedIterator.java +++ b/src/java/org/apache/cassandra/db/columniterator/SSTableReversedIterator.java @@ -426,7 +426,7 @@ public class SSTableReversedIterator extends AbstractSSTableIterator public void reset() { built = null; - rowBuilder = BTree.builder(metadata.comparator); + rowBuilder.reuse(); deletionBuilder = MutableDeletionInfo.builder(partitionLevelDeletion, metadata().comparator, false); } diff --git a/src/java/org/apache/cassandra/db/rows/BTreeRow.java b/src/java/org/apache/cassandra/db/rows/BTreeRow.java index 15ac30a8d5..c70e0e2cfb 100644 --- a/src/java/org/apache/cassandra/db/rows/BTreeRow.java +++ b/src/java/org/apache/cassandra/db/rows/BTreeRow.java @@ -738,7 +738,7 @@ public class BTreeRow extends AbstractRow this.clustering = null; this.primaryKeyLivenessInfo = LivenessInfo.EMPTY; this.deletion = Deletion.LIVE; - this.cells_ = null; + this.cells_.reuse(); this.hasComplex = false; } diff --git a/src/java/org/apache/cassandra/db/rows/ComplexColumnData.java b/src/java/org/apache/cassandra/db/rows/ComplexColumnData.java index 380af7abac..1395782c9a 100644 --- a/src/java/org/apache/cassandra/db/rows/ComplexColumnData.java +++ b/src/java/org/apache/cassandra/db/rows/ComplexColumnData.java @@ -242,7 +242,10 @@ public class ComplexColumnData extends ColumnData implements Iterable { this.column = column; this.complexDeletion = DeletionTime.LIVE; // default if writeComplexDeletion is not called - this.builder = BTree.builder(column.cellComparator()); + if (builder == null) + builder = BTree.builder(column.cellComparator()); + else + builder.reuse(column.cellComparator()); } public void addComplexDeletion(DeletionTime complexDeletion) diff --git a/src/java/org/apache/cassandra/utils/btree/BTree.java b/src/java/org/apache/cassandra/utils/btree/BTree.java index a4519b9451..a6c9826810 100644 --- a/src/java/org/apache/cassandra/utils/btree/BTree.java +++ b/src/java/org/apache/cassandra/utils/btree/BTree.java @@ -21,12 +21,12 @@ package org.apache.cassandra.utils.btree; import java.util.*; import java.util.function.Consumer; +import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Function; import com.google.common.base.Predicate; import com.google.common.collect.Iterators; import com.google.common.collect.Ordering; -import io.netty.util.Recycler; import org.apache.cassandra.utils.ObjectSizes; import static com.google.common.collect.Iterables.concat; @@ -769,25 +769,14 @@ public class BTree return 1 + lookupSizeMap(root, childIndex - 1); } - final static Recycler builderRecycler = new Recycler() - { - protected Builder newObject(Handle handle) - { - return new Builder(handle); - } - }; - public static Builder builder(Comparator comparator) { - Builder builder = builderRecycler.get(); - builder.reuse(comparator); - - return builder; + return new Builder<>(comparator); } public static Builder builder(Comparator comparator, int initialCapacity) { - return builder(comparator); + return new Builder<>(comparator, initialCapacity); } public static class Builder @@ -816,12 +805,23 @@ public class BTree boolean detected = true; // true if we have managed to cheaply ensure sorted (+ filtered, if resolver == null) as we have added boolean auto = true; // false if the user has promised to enforce the sort order and resolve any duplicates QuickResolver quickResolver; - final Recycler.Handle recycleHandle; - - private Builder(Recycler.Handle handle) + protected Builder(Comparator comparator) + { + this(comparator, 16); + } + + protected Builder(Comparator comparator, int initialCapacity) + { + if (initialCapacity == 0) + initialCapacity = 16; + this.comparator = comparator; + this.values = new Object[initialCapacity]; + } + + @VisibleForTesting + public Builder() { - this.recycleHandle = handle; this.values = new Object[16]; } @@ -833,7 +833,6 @@ public class BTree this.detected = builder.detected; this.auto = builder.auto; this.quickResolver = builder.quickResolver; - this.recycleHandle = null; } /** @@ -851,30 +850,17 @@ public class BTree return this; } - public void recycle() + public void reuse() { - if (recycleHandle != null) - { - this.cleanup(); - builderRecycler.recycle(this, recycleHandle); - } + reuse(comparator); } - /** - * Cleans up the Builder instance before recycling it. - */ - private void cleanup() + public void reuse(Comparator comparator) { - quickResolver = null; + this.comparator = comparator; Arrays.fill(values, null); count = 0; detected = true; - auto = true; - } - - private void reuse(Comparator comparator) - { - this.comparator = comparator; } public Builder auto(boolean auto) @@ -1101,16 +1087,9 @@ public class BTree public Object[] build() { - try - { - if (auto) - autoEnforce(); - return BTree.build(Arrays.asList(values).subList(0, count), UpdateFunction.noOp()); - } - finally - { - this.recycle(); - } + if (auto) + autoEnforce(); + return BTree.build(Arrays.asList(values).subList(0, count), UpdateFunction.noOp()); } } diff --git a/test/microbench/org/apache/cassandra/test/microbench/BTreeBuildBench.java b/test/microbench/org/apache/cassandra/test/microbench/BTreeBuildBench.java new file mode 100644 index 0000000000..0d89ebbbc0 --- /dev/null +++ b/test/microbench/org/apache/cassandra/test/microbench/BTreeBuildBench.java @@ -0,0 +1,96 @@ +/* + * 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.util.ArrayList; +import java.util.Comparator; +import java.util.List; +import java.util.concurrent.TimeUnit; + +import org.apache.cassandra.utils.btree.BTree; +import org.apache.cassandra.utils.btree.UpdateFunction; +import org.openjdk.jmh.annotations.Benchmark; +import org.openjdk.jmh.annotations.BenchmarkMode; +import org.openjdk.jmh.annotations.Fork; +import org.openjdk.jmh.annotations.Level; +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; + +@BenchmarkMode(Mode.Throughput) +@OutputTimeUnit(TimeUnit.MILLISECONDS) +@Warmup(iterations = 4, time = 1, timeUnit = TimeUnit.SECONDS) +@Measurement(iterations = 8, time = 2, timeUnit = TimeUnit.SECONDS) +@Fork(value = 2) +@Threads(4) +@State(Scope.Benchmark) +public class BTreeBuildBench +{ + private List data; + + @Param({"1", "2", "5", "10", "20", "40", "100", "1000", "10000", "100000"}) + int dataSize; + + private static final Comparator CMP = new Comparator() + { + public int compare(Integer o1, Integer o2) + { + return Integer.compare(o1, o2); + } + }; + + @Setup(Level.Trial) + public void setup() + { + data = new ArrayList<>(dataSize); + for (int i = 0 ; i < dataSize; i++) + data.add(i); + } + + private int buildTree(List data) + { + Object[] btree = BTree.build(data, UpdateFunction.noOp()); + // access the btree to avoid java optimized out this code + return BTree.size(btree); + } + + private int treeBuilderAddAll(List data) + { + BTree.Builder builder = BTree.builder(Comparator.naturalOrder()); + Object[] btree = builder.addAll(data).build(); + return BTree.size(btree); + } + + @Benchmark + public int treeBuilderRecycleAdd() + { + BTree.Builder builder = BTree.builder(Comparator.naturalOrder()); + builder.auto(false); + for (Integer v : data) + builder.add(v); + Object[] btree = builder.build(); + return BTree.size(btree); + } +} diff --git a/test/unit/org/apache/cassandra/utils/BTreeTest.java b/test/unit/org/apache/cassandra/utils/BTreeTest.java index ec4cdb8f7f..9a59e3aae2 100644 --- a/test/unit/org/apache/cassandra/utils/BTreeTest.java +++ b/test/unit/org/apache/cassandra/utils/BTreeTest.java @@ -261,6 +261,32 @@ public class BTreeTest } } + @Test + public void testBuilderReuse() + { + List sorted = seq(20); + BTree.Builder builder = BTree.builder(Comparator.naturalOrder()); + builder.auto(false); + for (int i : sorted) + builder.add(i); + checkResult(20, builder.build()); + + builder.reuse(); + assertTrue(builder.build() == BTree.empty()); + for (int i = 0; i < 12; i++) + builder.add(sorted.get(i)); + checkResult(12, builder.build()); + + builder.auto(true); + builder.reuse(Comparator.reverseOrder()); + for (int i = 0; i < 12; i++) + builder.add(sorted.get(i)); + checkResult(12, builder.build(), BTree.Dir.DESC); + + builder.reuse(); + assertTrue(builder.build() == BTree.empty()); + } + private static class Accumulator extends Number implements Comparable { final int base; @@ -385,7 +411,12 @@ public class BTreeTest private static void checkResult(int count, Object[] btree) { - Iterator iter = BTree.slice(btree, CMP, BTree.Dir.ASC); + checkResult(count, btree, BTree.Dir.ASC); + } + + private static void checkResult(int count, Object[] btree, BTree.Dir dir) + { + Iterator iter = BTree.slice(btree, CMP, dir); int i = 0; while (iter.hasNext()) assertEquals(iter.next(), ints[i++]);