Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add ValidateString option into decode #253

Merged
merged 30 commits into from Aug 11, 2022
Merged

add ValidateString option into decode #253

merged 30 commits into from Aug 11, 2022

Conversation

liuq19
Copy link
Collaborator

@liuq19 liuq19 commented Jun 30, 2022

MR 描述

为 unmarshal/decode 添加了 ValidateString 的可选项。开启后,decode 的时候可以校验 未转义的控制字符 (如 "\x00" )、检验 非法 utf-8 字符(如 "\xff")。

性能测试

不开 ValidateString 选项,无明显劣化

测试命令: ../bench.py -b "\"BenchmarkUnmarshalInterface/GolangSource_Sonic|BenchmarkUnmarshalConcrete/TwitterStatus_Sonic\"" -t 1000x -c

测试结果:

go get golang.org/x/perf/cmd/benchstat && go install golang.org/x/perf/cmd/benchstat && benchstat -sort=delta /tmp/tmpuxbn9s3h.main.txt /tmp/tmpm4go9ktd.target.txt
go: added golang.org/x/perf v0.0.0-20220722155240-3d85ee92886d
name                                          old time/op    new time/op    delta
UnmarshalConcrete/TwitterStatus_Sonic-256       2.55ms ± 6%    2.56ms ±10%    ~     (p=0.888 n=8+9)
UnmarshalConcrete/TwitterStatus_SonicStd-256    2.96ms ± 9%    3.04ms ± 5%    ~     (p=0.075 n=10+10)
UnmarshalInterface/GolangSource_Sonic-256       16.6ms ± 3%    16.7ms ± 1%    ~     (p=0.280 n=10+10)
UnmarshalInterface/GolangSource_SonicStd-256    20.1ms ± 3%    20.3ms ± 2%    ~     (p=0.968 n=10+9)

name                                          old speed      new speed      delta
UnmarshalConcrete/TwitterStatus_Sonic-256      248MB/s ± 6%   248MB/s ± 9%    ~     (p=0.888 n=8+9)
UnmarshalConcrete/TwitterStatus_SonicStd-256   214MB/s ± 9%   208MB/s ± 5%    ~     (p=0.075 n=10+10)
UnmarshalInterface/GolangSource_Sonic-256      117MB/s ± 3%   116MB/s ± 1%    ~     (p=0.256 n=10+10)
UnmarshalInterface/GolangSource_SonicStd-256  96.4MB/s ± 3%  95.6MB/s ± 2%    ~     (p=0.951 n=10+9)

name                                          old alloc/op   new alloc/op   delta
UnmarshalInterface/GolangSource_SonicStd-256    12.2MB ± 0%    12.2MB ± 0%  +0.13%  (p=0.003 n=9+10)
UnmarshalConcrete/TwitterStatus_Sonic-256       1.96MB ± 0%    1.96MB ± 1%    ~     (p=0.356 n=9+10)
UnmarshalConcrete/TwitterStatus_SonicStd-256    2.13MB ± 0%    2.13MB ± 0%    ~     (p=0.579 n=10+10)
UnmarshalInterface/GolangSource_Sonic-256       11.1MB ± 0%    11.1MB ± 0%    ~     (p=0.460 n=8+10)

name                                          old allocs/op  new allocs/op  delta
UnmarshalInterface/GolangSource_Sonic-256         128k ± 0%      128k ± 0%  +0.00%  (p=0.001 n=10+10)
UnmarshalConcrete/TwitterStatus_Sonic-256        1.56k ± 0%     1.56k ± 0%    ~     (all equal)
UnmarshalConcrete/TwitterStatus_SonicStd-256     5.62k ± 0%     5.62k ± 0%    ~     (all equal)
UnmarshalInterface/GolangSource_SonicStd-256      231k ± 0%      231k ± 0%  -0.00%  (p=0.025 n=10+9)

开 ValidateString 后,性能劣化约 2~3%

测试命令: ../bench.py -b "\"BenchmarkUnmarshalInterface/GolangSource_Sonic|BenchmarkUnmarshalConcrete/TwitterStatus_Sonic\"" -t 1000x -c

测试结果:

go get golang.org/x/perf/cmd/benchstat && go install golang.org/x/perf/cmd/benchstat && benchstat -sort=delta /tmp/tmpie1ig43z.main.txt /tmp/tmphz28kbx6.target.txt
go: added golang.org/x/perf v0.0.0-20220722155240-3d85ee92886d
name                                          old time/op    new time/op    delta
UnmarshalInterface/GolangSource_Sonic-256       16.5ms ± 1%    17.1ms ± 1%  +3.76%  (p=0.000 n=10+10)
UnmarshalInterface/GolangSource_SonicStd-256    20.2ms ± 1%    20.7ms ± 1%  +2.57%  (p=0.000 n=10+10)
UnmarshalConcrete/TwitterStatus_Sonic-256       2.51ms ± 4%    2.46ms ±13%    ~     (p=0.829 n=8+10)
UnmarshalConcrete/TwitterStatus_SonicStd-256    2.99ms ± 4%    3.06ms ± 8%    ~     (p=0.218 n=10+10)

name                                          old speed      new speed      delta
UnmarshalInterface/GolangSource_Sonic-256      118MB/s ± 1%   113MB/s ± 1%  -3.63%  (p=0.000 n=10+10)
UnmarshalInterface/GolangSource_SonicStd-256  96.3MB/s ± 1%  93.9MB/s ± 1%  -2.51%  (p=0.000 n=10+10)
UnmarshalConcrete/TwitterStatus_Sonic-256      252MB/s ± 4%   258MB/s ±15%    ~     (p=0.829 n=8+10)
UnmarshalConcrete/TwitterStatus_SonicStd-256   212MB/s ± 4%   207MB/s ± 9%    ~     (p=0.218 n=10+10)

name                                          old alloc/op   new alloc/op   delta
UnmarshalConcrete/TwitterStatus_Sonic-256       1.96MB ± 1%    1.96MB ± 1%    ~     (p=0.315 n=10+10)
UnmarshalConcrete/TwitterStatus_SonicStd-256    2.13MB ± 1%    2.12MB ± 1%    ~     (p=0.739 n=10+10)
UnmarshalInterface/GolangSource_Sonic-256       11.1MB ± 0%    11.1MB ± 0%    ~     (p=0.739 n=10+10)
UnmarshalInterface/GolangSource_SonicStd-256    12.2MB ± 0%    12.2MB ± 0%    ~     (p=0.190 n=10+10)

name                                          old allocs/op  new allocs/op  delta
UnmarshalConcrete/TwitterStatus_SonicStd-256     5.62k ± 0%     5.62k ± 0%  +0.02%  (p=0.000 n=10+9)
UnmarshalInterface/GolangSource_Sonic-256         128k ± 0%      128k ± 0%  +0.00%  (p=0.000 n=9+7)
UnmarshalInterface/GolangSource_SonicStd-256      231k ± 0%      231k ± 0%  +0.00%  (p=0.000 n=9+9)
UnmarshalConcrete/TwitterStatus_Sonic-256        1.55k ± 0%     1.56k ± 0%    ~     (p=0.294 n=8+10)
git checkout -- .
git checkout fix/add_validate

@AsterDY AsterDY marked this pull request as ready for review August 10, 2022 07:20
@liuq19 liuq19 changed the title add ValidString option into decode add ValidateString option into decode Aug 11, 2022
AsterDY
AsterDY previously approved these changes Aug 11, 2022
@AsterDY AsterDY enabled auto-merge (squash) August 11, 2022 11:05
@AsterDY AsterDY merged commit de2dc2c into main Aug 11, 2022
@AsterDY AsterDY deleted the fix/add_validate branch August 11, 2022 11:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants