[CH] Optimize aggregate state serialization performance #3279

liuneng1994 · 2023-09-26T02:01:00Z

What changes were proposed in this pull request?

Optimize aggregate state serialization performance, convert fixed size agg state to fixed string,
convert variable size aggregation state to string.

How was this patch tested?

(Please explain how this patch was tested. E.g. unit tests, integration tests, manual tests)

(If this patch involves UI changes, please attach a screenshot; otherwise, remove this)

github-actions · 2023-09-26T02:01:19Z

Thanks for opening a pull request!

Could you open an issue for this pull request on Github Issues?

https://github.com/oap-project/gluten/issues

Then could you also rename commit message and pull request title in the following format?

[GLUTEN-${ISSUES_ID}][COMPONENT]feat/fix: ${detailed message}

See also:

Other pull requests

github-actions · 2023-09-26T02:01:34Z

Run Gluten Clickhouse CI

github-actions · 2023-09-26T08:49:45Z

Run Gluten Clickhouse CI

baibaichen · 2023-09-27T02:05:47Z

--conf spark.sql.autoBroadcastJoinThreshold=100MB
HDSF
TPCH 100

cpp-ch/local-engine/Builder/SerializedPlanBuilder.cpp

lgbo-ustc · 2023-09-27T02:22:44Z

cpp-ch/local-engine/Storages/IO/AggregateSerializationUtils.cpp

+        return col;
+    }
+    const auto *aggregate_col = checkAndGetColumn<ColumnAggregateFunction>(*col.column);
+    size_t state_size = aggregate_col->getAggregateFunction()->sizeOfData();


Is the state size of max(String) and collect_list(x) fixed?

exclude collect_list, max(String) will use sort aggregate and fallback now

cpp-ch/local-engine/Storages/IO/AggregateSerializationUtils.cpp

lgbo-ustc · 2023-09-27T02:34:52Z

cpp-ch/local-engine/Storages/IO/NativeWriter.cpp

+}
+
+size_t NativeWriter::write(const DB::Block & block)
+{


what is the purpose of IndexOfBlockForNativeFormat in the original NativeWriter

some index data..useless for us

github-actions · 2023-09-27T12:18:38Z

Run Gluten Clickhouse CI

github-actions · 2023-09-28T08:14:38Z

Run Gluten Clickhouse CI

github-actions · 2023-09-28T09:00:14Z

Run Gluten Clickhouse CI

zhanglistar · 2023-10-07T09:17:57Z

cpp-ch/local-engine/Storages/IO/NativeWriter.cpp

+{
+    /** If there are columns-constants - then we materialize them.
+      * (Since the data type does not know how to serialize / deserialize constants.)
+      */


Maybe we can design some protocol to write less data for constant column to reducing IO amount.

这个是CH copy过来的。。暂时不做优化

zhanglistar · 2023-10-08T02:22:26Z

cpp-ch/local-engine/Storages/IO/AggregateSerializationUtils.cpp

+bool isFixedSizeStateAggregateFunction(const String& name)
+{
+    // TODO max(String) should exclude, but fallback now
+    static const std::set<String> function_set = {"min", "max", "sum", "count", "avg"};


According https://spark.apache.org/docs/latest/sql-ref-functions-builtin.html, maybe other functions can also be added to the set, like mean.

这里我会换一个方案来同时支持定长和变长的聚合

目前先用这个，后面遇到了在更新

github-actions · 2023-10-31T01:58:42Z

Run Gluten Clickhouse CI

github-actions · 2023-10-31T02:35:54Z

Run Gluten Clickhouse CI

github-actions · 2023-10-31T03:20:41Z

Run Gluten Clickhouse CI

github-actions · 2023-10-31T07:24:54Z

Run Gluten Clickhouse CI

github-actions · 2023-11-02T07:41:22Z

Run Gluten Clickhouse CI

github-actions · 2023-11-03T08:35:49Z

Run Gluten Clickhouse CI

github-actions · 2023-11-03T09:47:35Z

Run Gluten Clickhouse CI

github-actions · 2023-11-05T14:41:14Z

Run Gluten Clickhouse CI

zzcclp · 2023-11-14T02:29:55Z

...32/src/test/scala/org/apache/spark/sql/execution/adaptive/GlutenAdaptiveQueryExecSuite.scala

@@ -438,8 +438,10 @@ class GlutenAdaptiveQueryExecSuite extends AdaptiveQueryExecSuite with GlutenSQL
  test("gluten Exchange reuse") {
    withSQLConf(
      SQLConf.ADAPTIVE_EXECUTION_ENABLED.key -> "true",
-      SQLConf.AUTO_BROADCASTJOIN_THRESHOLD.key -> "100",
-      SQLConf.SHUFFLE_PARTITIONS.key -> "5") {
+      // magic threshold, ch backend has two bhj when threshold is 100


@PHILO-HE @rui-mo please help to check, thanks.

...33/src/test/scala/org/apache/spark/sql/execution/adaptive/GlutenAdaptiveQueryExecSuite.scala

lgbo-ustc · 2023-11-14T02:39:16Z

cpp-ch/local-engine/Storages/IO/AggregateSerializationUtils.cpp

+    size_t state_size = aggregate_col->getAggregateFunction()->sizeOfData();
+    auto res_type = std::make_shared<DataTypeFixedString>(state_size);
+    auto res_col = res_type->createColumn();
+    PaddedPODArray<UInt8> & column_chars_t = assert_cast<ColumnFixedString &>(*res_col).getChars();


use ColumnFixedString::reserve is better

lgbo-ustc · 2023-11-14T02:41:21Z

cpp-ch/local-engine/Storages/IO/AggregateSerializationUtils.cpp

+    for (const auto & item : aggregate_col->getData())
+    {
+        column_chars_t.insert_assume_reserved(item, item + state_size);
+    }


for small state objects, could we try to use memcpy ?

too much function call may have some performance issue

这个写法是参考arrowColumnToCHColumn的readColumnWithNumericData。应该不会有性能问题

github-actions · 2023-11-14T08:46:35Z

Run Gluten Clickhouse CI

github-actions · 2023-11-14T09:01:51Z

Run Gluten Clickhouse CI

github-actions · 2023-11-14T09:14:39Z

Run Gluten Clickhouse CI

lgbo-ustc · 2023-11-14T09:59:58Z

LGTM

github-actions · 2023-11-14T10:36:47Z

Run Gluten Clickhouse CI

taiyang-li · 2023-11-14T12:10:07Z

cpp-ch/local-engine/Storages/IO/AggregateSerializationUtils.cpp

+    return isFixedSizeStateAggregateFunction(function->getName()) && isFixedSizeArguments(function->getArgumentTypes());
+}
+
+DB::ColumnWithTypeAndName convertAggregateStateToFixedString(DB::ColumnWithTypeAndName col)


尽量避免传值，避免不必要的复制开销。

taiyang-li · 2023-11-14T12:10:21Z

cpp-ch/local-engine/Storages/IO/AggregateSerializationUtils.cpp

+    }
+    return DB::ColumnWithTypeAndName(std::move(res_col), type, col.name);
+}
+DB::Block convertAggregateStateInBlock(DB::Block block)


避免传值

taiyang-li · 2023-11-14T12:14:35Z

cpp-ch/local-engine/Storages/IO/AggregateSerializationUtils.cpp

+
+bool isFixedSizeArguments(DataTypes data_types)
+{
+    return data_types.front()->isValueRepresentedByNumber();


isValueUnambiguouslyRepresentedInFixedSizeContiguousMemoryRegion
使用这个接口是否更为准确。

先限制在数字类型，其他类型没测试过

taiyang-li · 2023-11-14T12:15:13Z

cpp-ch/local-engine/Storages/IO/AggregateSerializationUtils.cpp

+}
+DB::Block convertAggregateStateInBlock(DB::Block block)
+{
+    ColumnsWithTypeAndName columns;


加上reserve

taiyang-li · 2023-11-14T12:16:12Z

cpp-ch/local-engine/Storages/IO/AggregateSerializationUtils.cpp

+    return data_types.front()->isValueRepresentedByNumber();
+}
+
+bool isFixedSizeAggregateFunction(DB::AggregateFunctionPtr function)


避免传值，其他类似地方也可以优化下。

taiyang-li · 2023-11-14T12:19:09Z

cpp-ch/local-engine/Storages/IO/AggregateSerializationUtils.cpp

+
+DB::ColumnWithTypeAndName convertAggregateStateToFixedString(DB::ColumnWithTypeAndName col)
+{
+    if (!WhichDataType(col.type).isAggregateFunction())


这里的判断可以去掉，改成下面判断aggregate_col是否为nullptr

这种类型AggregateFunction(sum, Nullable(Int64)) 是否可以走fixed string
现在实际上是走的变长string.

bug, 需要remove nullable

taiyang-li · 2023-11-14T12:20:01Z

cpp-ch/local-engine/Storages/IO/AggregateSerializationUtils.cpp

+    auto res_type = std::make_shared<DataTypeString>();
+    auto res_col = res_type->createColumn();
+    PaddedPODArray<UInt8> & column_chars = assert_cast<ColumnString &>(*res_col).getChars();
+    column_chars.reserve(aggregate_col->size() * 60);


60这个数字是怎么来的？会不会导致分配超出实际需要的内存

测试代码，删了

github-actions · 2023-11-15T04:03:00Z

Run Gluten Clickhouse CI

zhanglistar · 2023-11-15T08:32:00Z

cpp-ch/local-engine/Storages/IO/AggregateSerializationUtils.cpp

 }

-bool isFixedSizeAggregateFunction(DB::AggregateFunctionPtr function)
+bool isFixedSizeAggregateFunction(const DB::AggregateFunctionPtr& function)


shared_ptr
不用传引用吧直接传值

就这样吧，没有太大区别

zhanglistar · 2023-11-15T08:41:15Z

cpp-ch/local-engine/Storages/IO/AggregateSerializationUtils.cpp

+    for (const auto & item : aggregate_col->getData())
+    {
+        aggregate_col->getAggregateFunction()->serialize(item, value_writer);
+        writeChar('\0', value_writer);


这个有必要？

字符串需要\0结尾，必须的

github-actions · 2023-11-16T01:37:20Z

Run Gluten Clickhouse CI

lgbo-ustc · 2023-11-16T07:13:59Z

LGTM

taiyang-li · 2023-11-16T08:17:48Z

LGTM

baibaichen

cool, let's merge

lgbo-ustc reviewed Sep 27, 2023

View reviewed changes

cpp-ch/local-engine/Storages/IO/AggregateSerializationUtils.cpp Outdated Show resolved Hide resolved

lgbo-ustc reviewed Sep 27, 2023

View reviewed changes

liuneng1994 force-pushed the optimize-agg-state-serialize branch from bc8c977 to bde57b9 Compare September 27, 2023 12:18

zhanglistar reviewed Oct 7, 2023

View reviewed changes

zhanglistar reviewed Oct 8, 2023

View reviewed changes

liuneng1994 force-pushed the optimize-agg-state-serialize branch from 1f50efc to de46b01 Compare October 31, 2023 01:58

liuneng1994 force-pushed the optimize-agg-state-serialize branch from f3dd446 to c466268 Compare October 31, 2023 03:20

liuneng1994 force-pushed the optimize-agg-state-serialize branch from 83b85ba to de32d27 Compare November 3, 2023 09:47

liuneng1994 force-pushed the optimize-agg-state-serialize branch from de32d27 to e0e8249 Compare November 5, 2023 14:40

liuneng1994 changed the title ~~[WIP] [CH] Optimize aggregate state serialization performance~~ [CH] Optimize aggregate state serialization performance Nov 6, 2023

zzcclp reviewed Nov 14, 2023

View reviewed changes

...33/src/test/scala/org/apache/spark/sql/execution/adaptive/GlutenAdaptiveQueryExecSuite.scala Show resolved Hide resolved

lgbo-ustc reviewed Nov 14, 2023

View reviewed changes

liuneng1994 force-pushed the optimize-agg-state-serialize branch from e0e8249 to 7e54520 Compare November 14, 2023 08:46

taiyang-li requested changes Nov 14, 2023

View reviewed changes

zhanglistar reviewed Nov 15, 2023

View reviewed changes

liuneng1994 added 14 commits November 16, 2023 09:36

convert agg type to fixed string

f305598

optimize shuffle read

d16d559

fix bug

95d7e5b

add white list for agg opt

71e4a0b

fix style

ffc034a

optimize shuffle more

c285d69

combine two optimization

739377c

fix performance

f59a09f

fix ut failed

0b4ef63

Update AggregateSerializationUtils.cpp

431272d

fix

d329ff8

fix

af0e256

update

db955e4

some fix

80ec518

baibaichen force-pushed the optimize-agg-state-serialize branch from cfa0a4f to 80ec518 Compare November 16, 2023 01:36

baibaichen approved these changes Nov 16, 2023

View reviewed changes

baibaichen merged commit aab6dbe into apache:main Nov 16, 2023
17 checks passed

[CH] Optimize aggregate state serialization performance #3279

[CH] Optimize aggregate state serialization performance #3279

Conversation

liuneng1994 commented Sep 26, 2023 • edited Loading

What changes were proposed in this pull request?

How was this patch tested?

github-actions bot commented Sep 26, 2023

github-actions bot commented Sep 26, 2023

github-actions bot commented Sep 26, 2023

baibaichen commented Sep 27, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Sep 27, 2023

github-actions bot commented Sep 28, 2023

github-actions bot commented Sep 28, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Oct 31, 2023

github-actions bot commented Oct 31, 2023

github-actions bot commented Oct 31, 2023

github-actions bot commented Oct 31, 2023

github-actions bot commented Nov 2, 2023

github-actions bot commented Nov 3, 2023

github-actions bot commented Nov 3, 2023

github-actions bot commented Nov 5, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Nov 14, 2023

github-actions bot commented Nov 14, 2023

github-actions bot commented Nov 14, 2023

lgbo-ustc commented Nov 14, 2023

github-actions bot commented Nov 14, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

taiyang-li Nov 14, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Nov 15, 2023

zhanglistar Nov 15, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Nov 16, 2023

lgbo-ustc commented Nov 16, 2023

taiyang-li commented Nov 16, 2023

baibaichen left a comment

Choose a reason for hiding this comment

liuneng1994 commented Sep 26, 2023 •

edited

Loading

taiyang-li Nov 14, 2023 •

edited

Loading

zhanglistar Nov 15, 2023 •

edited

Loading