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

Optimize parser by using nested parser state #41

Merged
merged 2 commits into from
Jun 23, 2021

Conversation

zuiderkwast
Copy link
Collaborator

When parsing a nested array in chunks efficiently, the parser state
(continuation data) needs to be a nested structure. The parser is
restructured.

Fixes wooga#127

@zuiderkwast zuiderkwast requested a review from bjosv June 22, 2021 11:37
When parsing a nested array in chunks efficiently, the parser state
(continuation data) needs to be a nested structure. The parser is
restructured.

Fixes wooga#127
Copy link
Collaborator

@bjosv bjosv left a comment

Choose a reason for hiding this comment

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

LGTM, how is the cprof runs as in wooga#127 (comment) turning out with this change?

@zuiderkwast
Copy link
Collaborator Author

LGTM, how is the cprof runs as in wooga#127 (comment) turning out with this change?

Erlang/OTP 23 [erts-11.1.8] [source] [64-bit] [smp:12:12] [ds:12:12:10] [async-threads:1]

Eshell V11.1.8  (abort with ^G)
1> {ok, E} = eredis:start_link().     
{ok,<0.233.0>}
2> eredis:q(E,["FLUSHDB"]).
{ok,<<"OK">>}
3> A = binary:copy(<<"a">>, 10000000).
<<"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"...>>
4> eredis:q(E,["HSET", "h", "1", A]).
{ok,<<"1">>}
5> cprof:start(), R=eredis:q(E, ["HGETALL", "h"], 100000), cprof:pause(),R.
{ok,[<<"1">>,
     <<"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"...>>]}
6> cprof:analyse(eredis_parser).
{eredis_parser,20571,
               [{{eredis_parser,do_parse,2},6860},
                {{eredis_parser,return,2},6854},
                {{eredis_parser,parse,2},6854},
                {{eredis_parser,split_by_newline,2},3}]}
7> cprof:stop().
15886
8> timer:tc(fun() -> eredis:q(E, ["HGETALL", "h"], 100000) end).
{21970,
 {ok,[<<"1">>,
      <<"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"...>>]}}
9> timer:tc(fun() -> eredis:q(E, ["HGET", "h", "1"]) end).      
{20421,
 {ok,<<"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"...>>}}

Compared to wooga/eredis, there are fewer function calls and is faster also for the fast example. Tests done on my laptop. Times in microseconds.

Command wooga/eredis This PR
HGET h 1 25117 20421
HGETALL h 9429598 21970

Copy link
Collaborator

@bjosv bjosv left a comment

Choose a reason for hiding this comment

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

Nice

@zuiderkwast zuiderkwast merged commit adaa51f into Nordix:master Jun 23, 2021
@zuiderkwast zuiderkwast deleted the nested-parser-state branch June 23, 2021 13:14
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.

Poor performance of Multi Bulk response parser
2 participants