row base serializer #5791

youngsofun · 2022-06-06T03:31:20Z

I hereby agree to the terms of the CLA available at: https://databend.rs/dev/policies/cla/

Summary

add new trait and trait functions so I can replace them increasingly

use row-based serializer
HTTP handler return string fields
serialization problem with "f32 as f64" #5863

Conclusions of prev bench results:

Compared with row base impl in this pr:

Return vec is very slow
Return something like StringColumn is faster but still slow(x 2) and has more memory overhead
Return streamin_iter is a bit complex and a litter slower (x1.5 for i32 len 1024, better for longger column )

with row base impl

holding &[T] in Serializer make it 30% faster(for i32 column), compared with passing ColumnRef each time and cast
enum_dispath is 10% faster than dyn dispatch

@sundy-li if you vote for enum_dispath, I will delete the dyn dispatch code, and go on with other types

some notes about serializer:

add ConstSerializer, it can be optimized a lot later by caching the serialized value
writes string to MySQL writer for float (instead of writing float and let mysql-srv format it), to make float formats consistent (e.g. float in array use our own serializer,)， its the only place need to change the test. see the 2nd commit.
- lexical-core did not trim .0 of float by default unless use it with WriteFloatOptions, since we now use the same float str format in all output formats (need more effort to specialize for each format). it is better to keep .0 to make it easy for other applicaitons to detect the schema for json, csv .etc.

Changelog

Performance Improvement

Related Issues

Fixes #issue

vercel · 2022-06-06T03:31:25Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Ignored Deployment

Name	Status	Preview	Updated
databend	⬜️ Ignored (Inspect)		Jun 9, 2022 at 7:26AM (UTC)

mergify · 2022-06-06T03:31:32Z

Thanks for the contribution!
I have applied any labels matching special text in your PR Changelog.

Please review the labels and make any necessary changes.

youngsofun · 2022-06-06T08:20:34Z

enum_dispath is not IDE friendly, both do not support associate const( can be used in quote), not sure if enum_dispath can be able to support it later, is there any other drawback on it， since it is only 10% faster?

@sundy-li

common/datavalues/src/types/serializations_new/boolean.rs

BohuTANG · 2022-06-07T13:44:09Z

@mergify update

mergify · 2022-06-07T13:44:48Z

update

✅ Branch has been successfully updated

sundy-li · 2022-06-07T13:45:44Z

enum_dispath is not IDE friendly, both do not support associate const( can be used in quote), not sure if enum_dispath can be able to support it later, is there any other drawback on it， since it is only 10% faster?

Let's make life easier, enum_dispatch is not needed.

youngsofun · 2022-06-08T04:05:12Z

enum_dispath is not IDE friendly, both do not support associate const( can be used in quote), not sure if enum_dispath can be able to support it later, is there any other drawback on it， since it is only 10% faster?

Let's make life easier, enum_dispatch is not needed.

the serializer is used in limited places, not too bothering, I prefer keep it for now

youngsofun · 2022-06-08T07:12:58Z

@BohuTANG @sundy-li is it acceptable to see 1.0 instead of 1 for float in mysql client?

sundy-li · 2022-06-08T07:41:40Z

table to see 1.0 instead of 1 f

https://www.db-fiddle.com/f/abKfoifJD29jCXVgBNnZap/0

MySQL outputs 1, but I think we do not need to follow MySQL strictly.

youngsofun · 2022-06-08T07:42:56Z

snowflake

youngsofun · 2022-06-08T07:59:55Z

I prefer always use 1.0

uniform format， make it much easier to write tests for diff format and handlers
easier to know the type.
to use 1 for MySQL , need lexical_core::write_with_options, instead of lexical_core::write, maybe a little slow

trim_float is not used for any prepared FORMATS in lib lexical, including MySQL

https://github.com/Alexhuszagh/rust-lexical/blob/main/lexical-write-float/src/options.rs#L1373

table to see 1.0 instead of 1 f

https://www.db-fiddle.com/f/abKfoifJD29jCXVgBNnZap/0

MySQL outputs 1, but I think we do not need to follow MySQL strictly.

youngsofun · 2022-06-08T12:29:02Z

have some problem to use WriteFloatOptions

N::Options == WriteFloatOptions only holds for f32 and f64

pub fn write_with_options<'a, N: ToLexicalWithOptions, const FORMAT: u128>(
    n: N,
    bytes: &'a mut [u8],
    options: &N::Options,
) -> &'a mut [u8] {
    n.to_lexical_with_options::<FORMAT>(bytes, options)

common/datavalues/src/types/data_type.rs

zhang2014 · 2022-06-08T12:51:32Z

common/datavalues/src/types/data_type.rs

+        }
+    }
+
+    fn create_serializer_inner<'a>(&self, _col: &'a ColumnRef) -> Result<TypeSerializerImpl<'a>> {


Why need this?

used to for normal column( non-const)

create_serializer_inner use for impl
create_serializer as interface

youngsofun · 2022-06-08T15:22:08Z

oops, SQL logic tests take effect, need to update expect the results for floats too
no need to change the code
will be done tomorrow morning

sundy-li · 2022-06-09T00:59:55Z

common/datavalues/src/types/serializations/array.rs

        Ok(Arc::new(ArrayColumnData::create(inner_data, offsets)))
    }
+
+    fn serialize_json(&self, format: &FormatSettings) -> Result<Vec<Value>> {


So row based api not works on serialize_json ?

serializer can has API for both row based and col-based(clickhouse, json::Value)

format choose to use which

pub fn block_to_json_value( block: &DataBlock, format: &FormatSettings, ) -> Result<Vec<Vec<JsonValue>>> { let cols = block_to_json_value_columns(block, format)?; Ok(transpose(cols)) }

Since we need to transpose, so serialize_json is row based ?

I guess column-based may be faster?

But it's not zero alloc in the transpose function. So row based is faster in this case.

But it's not zero alloc in the transpose function. So row based is faster in this case.

make sense

may I refactor it later in another pr later, this pr is large and need to change new-added tests case, hope it to merge early.

also need add formats based on this

@zhang2014

thx to @zhang2014

youngsofun · 2022-06-09T06:29:56Z

@sundy-li @zhang2014 rebase only to include new test cases, the original commits are not changed

youngsofun · 2022-06-09T08:30:15Z

@sundy-li @zhang2014 all tests passed (except a build), please review

sundy-li · 2022-06-09T08:33:26Z

/LGTM

youngsofun requested a review from sundy-li June 6, 2022 03:31

databend-bot added the need-review label Jun 6, 2022

youngsofun marked this pull request as draft June 6, 2022 03:31

mergify bot added the pr-performance label Jun 6, 2022

youngsofun mentioned this pull request Jun 6, 2022

serializer without tmp alloc. #5451

Closed

5 tasks

sundy-li reviewed Jun 6, 2022

View reviewed changes

common/datavalues/src/types/serializations_new/boolean.rs Outdated Show resolved Hide resolved

youngsofun force-pushed the output branch 5 times, most recently from 8020827 to 79ca832 Compare June 7, 2022 08:40

youngsofun marked this pull request as ready for review June 7, 2022 09:07

youngsofun marked this pull request as draft June 7, 2022 16:34

youngsofun force-pushed the output branch 2 times, most recently from 8060016 to 289fec2 Compare June 8, 2022 03:24

youngsofun force-pushed the output branch 2 times, most recently from c95a8a9 to fddffdc Compare June 8, 2022 05:49

youngsofun marked this pull request as ready for review June 8, 2022 06:41

sundy-li requested a review from zhang2014 June 8, 2022 07:51

zhang2014 reviewed Jun 8, 2022

View reviewed changes

common/datavalues/src/types/data_type.rs Outdated Show resolved Hide resolved

zhang2014 reviewed Jun 8, 2022

View reviewed changes

sundy-li reviewed Jun 9, 2022

View reviewed changes

youngsofun force-pushed the output branch from 4d5691d to 3b05cd8 Compare June 9, 2022 04:36

youngsofun added 6 commits June 9, 2022 13:54

row-based serializer.

2eea3de

mysql writer write string for float.

3404056

refactor: merge create_serializer_const into create_serializer

a93d413

thx to @zhang2014

update logic test.

632e973

http handler support return string fields json.

818ddf3

up logic test to use string_fields fo http handler

8aef9e0

youngsofun force-pushed the output branch from 3b05cd8 to 5cc7ccb Compare June 9, 2022 06:05

youngsofun added 3 commits June 9, 2022 14:18

support set bool literal in FormatSetting.

0b32973

update logic test.

cb9a793

update new stateless test

8e551cf

youngsofun force-pushed the output branch from 5cc7ccb to 8e551cf Compare June 9, 2022 06:19

fix bool serialization.

dbf0478

youngsofun force-pushed the output branch from 4753c2a to dbf0478 Compare June 9, 2022 07:26

databend-bot approved these changes Jun 9, 2022

View reviewed changes

BohuTANG merged commit 04729d3 into databendlabs:main Jun 9, 2022

row base serializer #5791

row base serializer #5791

Uh oh!

Conversation

youngsofun commented Jun 6, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Conclusions of prev bench results:

some notes about serializer:

Changelog

Related Issues

Uh oh!

vercel bot commented Jun 6, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mergify bot commented Jun 6, 2022

Uh oh!

youngsofun commented Jun 6, 2022

Uh oh!

Uh oh!

BohuTANG commented Jun 7, 2022

Uh oh!

mergify bot commented Jun 7, 2022

✅ Branch has been successfully updated

Uh oh!

sundy-li commented Jun 7, 2022

Uh oh!

youngsofun commented Jun 8, 2022

Uh oh!

youngsofun commented Jun 8, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sundy-li commented Jun 8, 2022

Uh oh!

youngsofun commented Jun 8, 2022

Uh oh!

youngsofun commented Jun 8, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

youngsofun commented Jun 8, 2022

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

youngsofun commented Jun 8, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

youngsofun commented Jun 9, 2022

Uh oh!

youngsofun commented Jun 9, 2022

Uh oh!

sundy-li commented Jun 9, 2022

Uh oh!

Uh oh!

youngsofun commented Jun 6, 2022 •

edited

Loading

vercel bot commented Jun 6, 2022 •

edited

Loading

youngsofun commented Jun 8, 2022 •

edited

Loading

youngsofun commented Jun 8, 2022 •

edited

Loading

youngsofun commented Jun 8, 2022 •

edited

Loading