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

Digest calculated in batches of sizeof(size_t) #606

Draft
wants to merge 3 commits into
base: develop
Choose a base branch
from

Conversation

grisumbras
Copy link
Member

@grisumbras grisumbras commented Aug 8, 2021

Fixes #601

@grisumbras grisumbras mentioned this pull request Aug 8, 2021
@codecov
Copy link

codecov bot commented Aug 8, 2021

Codecov Report

Merging #606 (c291598) into develop (8957955) will decrease coverage by 0.05%.
The diff coverage is 80.76%.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #606      +/-   ##
===========================================
- Coverage    92.98%   92.94%   -0.05%     
===========================================
  Files           85       85              
  Lines         8058     8080      +22     
===========================================
+ Hits          7493     7510      +17     
- Misses         565      570       +5     
Impacted Files Coverage Δ
include/boost/json/detail/digest.hpp 82.75% <80.76%> (-17.25%) ⬇️

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8957955...c291598. Read the comment docs.

@grisumbras grisumbras force-pushed the batched-digest branch 2 times, most recently from f99afa3 to a97a76f Compare August 8, 2021 14:39
@cppalliance-bot
Copy link

@vinniefalco
Copy link
Member

What is the status of this?

@grisumbras
Copy link
Member Author

I've made 6 variants of this and benched them. Here are the results:
https://docs.google.com/spreadsheets/d/1WEfTLx5vEb5TduzzHt-OkAj6HnS4ku4Y4Y7C3YGIgc8/edit?usp=sharing
The 6th approach seems most promising, fairly small regression for a few tests and huge improvements for many tests. But my local benchmarking disagrees with CI as you can see on the picture posted above by the bot.

@vinniefalco
Copy link
Member

yeah the automated performance CI bot is not accurate

@cppalliance-bot
Copy link

@vinniefalco
Copy link
Member

FYI this is still on my radar, but I have to wait until I return to boost.http_proto.

@grisumbras grisumbras force-pushed the batched-digest branch 2 times, most recently from cf0143c to 9d848ed Compare June 15, 2023 14:04
@cppalliance-bot
Copy link

@cppalliance-bot
Copy link

@grisumbras grisumbras force-pushed the batched-digest branch 9 times, most recently from 0e823dc to e174db9 Compare June 15, 2023 20:43
@cppalliance-bot
Copy link

@grisumbras grisumbras force-pushed the batched-digest branch 4 times, most recently from 84d8b73 to 3f4f931 Compare June 16, 2023 08:04
@cppalliance-bot
Copy link

@grisumbras grisumbras force-pushed the batched-digest branch 12 times, most recently from 8ae0c09 to 12fcc52 Compare June 16, 2023 13:13
This is intended to increase performance
@grisumbras grisumbras force-pushed the batched-digest branch 2 times, most recently from 5baf791 to 632fea9 Compare June 16, 2023 14:44
@cppalliance-bot
Copy link

@cppalliance-bot
Copy link

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.

Faster digest
3 participants