Skip to content

Eliminate looping transform in mcbp_parser::next #347

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

Merged
merged 5 commits into from
Jan 28, 2023
Merged

Conversation

davidkelly
Copy link
Contributor

I found that in mcbp_session_impl::do_read, 39% of the time was in mcbp::parser::next, with the vast majority of that in the std::transform
call after uncompressing the data. By using vector::insert instead,
that dropped to only 11% of the time in do:read.

Probably the upshot here is to look for everywhere we are filling containers with a back_inserter and considering a way to modify that to not loop and instead copy in blocks.

before:
image
after:
image

I found that in mcbp_session_impl::do_read, 39% of the time was in
mcbp::parser::next, with the vast majority of that in the std::transform
call after uncompressing the data.   By using vector::insert instead,
that dropped to only 11% of the time in do:read.

Probably the upshot here is to look for everywhere we are filling
containers with a back_inserter and considering a way to modify that to
not loop and instead copy in blocks.
@davidkelly
Copy link
Contributor Author

After doing same in the mcbp_parser::feed call:
image

@avsej
Copy link
Member

avsej commented Jan 27, 2023

cool, good catch!

@davidkelly
Copy link
Contributor Author

For upserts, etc... there was this same issue with couchbase::core::utils::json::generate_binary.

Before:
image

After:
image

@davidkelly davidkelly marked this pull request as ready for review January 27, 2023 21:52
@davidkelly davidkelly requested a review from avsej January 27, 2023 23:16
@avsej avsej merged commit d1e385a into main Jan 28, 2023
@avsej avsej deleted the dk/perf_parser_next branch January 28, 2023 14:33
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.

2 participants