Skip to content

Performance: Added fast_double_parser to improve double conversion time#683

Merged
filipecosta90 merged 7 commits into
masterfrom
fast.doubleparser
Apr 1, 2021
Merged

Performance: Added fast_double_parser to improve double conversion time#683
filipecosta90 merged 7 commits into
masterfrom
fast.doubleparser

Conversation

@filipecosta90

@filipecosta90 filipecosta90 commented Mar 23, 2021

Copy link
Copy Markdown
Contributor

This PR improves string to double conversion by leveraging a C Wrapper for the C++ Eisel-Lemire ParseFloat algorithm.
This optimizes primarily our write workflows and we've measured an improvement of ~=8% on the achievable ops/sec.

Bellow you can find the top-down tree of cpu usage that made us focus on this optimization.

Top-Down tree of hotspots by CPU utilization for TS.MADD....

Please notice the 16.7% line, which we're optimizing on this PR.

Function Stack CPU Time: Total CPU Time: Self Function (Full) Source File
add 96.10% 0.122s add module.c
RM_OpenKey 27.70% 0.066s RM_OpenKey module.c
internalAdd 24.80% 0.404s internalAdd module.c
RM_StringToDouble 16.70% 0.038s RM_StringToDouble module.c
RM_CloseKey 8.30% 0.114s RM_CloseKey module.c
RM_StringToLongLong 6.80% 0.082s RM_StringToLongLong module.c
zfree 4.20% 0.262s zfree zmalloc.c
ParseDuplicatePolicy 2.40% 0.184s ParseDuplicatePolicy query_language.c
RM_ModuleTypeGetType 1.90% 0.236s RM_ModuleTypeGetType module.c
_GI 1.50% 0.212s float _GI(long, int, bool, char) malloc.c

Validating improvements with a tsbs dataset

on a quick look, using a single shard to test we move from 1.4M datapoints/sec ( 140K ops/sec of TS.MADD with 10 points ) to 1.53M datapoints.

According to Eisel-Lemire ParseFloat algorithm numbers, we should see a 4X improvement on the string to double, which means we would move from 17% CPU time on conversion to 0.25*17%=4.25% on best case scenario. I'm seeing an real improvement of ~8-9% on ingestion (meaning we're at 2X improvement ) :

###########################
Standalone on master branch
###########################
Summary:
loaded 120960000 metrics in 85.206sec with 1 workers (mean rate 1419615.30 metrics/sec)
loaded 12097000 rows in 85.206sec with 1 workers (mean rate 141973.27 rows/sec)

###########################
Standalone on fast.doubleparser branch
###########################
Summary:
2021/03/23 20:08:17 finished benchmark
loaded 120960000 metrics in 78.765sec with 1 workers (mean rate 1535699.71 metrics/sec)
loaded 12097000 rows in 78.765sec with 1 workers (mean rate 153582.67 rows/sec)

@filipecosta90 filipecosta90 added the performance Performance related Issue/PR label Mar 23, 2021
@filipecosta90 filipecosta90 requested review from danni-m and rafie March 23, 2021 19:58
@filipecosta90 filipecosta90 changed the title [In-Progress] Added fast_double_parser to improve double conversion time [In-Progress] Performance: Added fast_double_parser to improve double conversion time Mar 23, 2021
@filipecosta90 filipecosta90 changed the title [In-Progress] Performance: Added fast_double_parser to improve double conversion time Performance: Added fast_double_parser to improve double conversion time Mar 23, 2021
@filipecosta90 filipecosta90 requested a review from gkorland March 23, 2021 22:45
danni-m
danni-m previously approved these changes Mar 29, 2021

@danni-m danni-m left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great to me

danni-m
danni-m previously approved these changes Apr 1, 2021
danni-m
danni-m previously approved these changes Apr 1, 2021
Comment thread src/Makefile Outdated

@rafie rafie left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lVgtm.
Few comments.
Also: don't worry about the dockerhub failure. It's a known issue when adding submodules.

Comment thread src/module.c Outdated
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

performance Performance related Issue/PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Performance: RM_StringToDouble takes **16.70%** of the total CPU time of TS.ADD/TS.MADD

3 participants