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.

Sign up for GitHub

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

Jump to bottom

Move get_block abi serialization off the main thread #696

Merged
merged 10 commits into from
Feb 15, 2023
Merged

Move get_block abi serialization off the main thread #696

merged 10 commits into from
Feb 15, 2023

Conversation

heifner
Copy link
Member

@heifner heifner commented Feb 7, 2023 •
edited
Loading

Move the abi serialization of signed_block for /v1/chain/get_block off the main thread. Also the abi cache used in this implementation actually provides a nice performance improvement. See comments on #677.

This PR also uses the min of --http-max-response-time-ms & --abi-serializer-max-time-ms for the deadline of get_block instead of running for the full --abi-serializer-max-time-ms before checking --http-max-response-time-ms.

Also includes reduction of string copies in abi_serializer.

Resolves #677

@heifner heifner changed the title Move abi serialization off the main thread for get_block Move get_block abi serialization off the main thread Feb 7, 2023
@heifner heifner requested a review from greg7mdp February 7, 2023 14:39
@heifner heifner changed the title Move get_block abi serialization off the main thread DRAFT: Move get_block abi serialization off the main thread Feb 7, 2023
@heifner heifner removed the request for review from greg7mdp February 7, 2023 15:07
@heifner heifner marked this pull request as draft February 7, 2023 15:07
@greg7mdp
Copy link
Contributor

greg7mdp commented Feb 7, 2023

Thanks for updating abi_serializer::set_abi to take a rvalue :-)!

@heifner heifner marked this pull request as ready for review February 7, 2023 18:25
@heifner heifner changed the title DRAFT: Move get_block abi serialization off the main thread Move get_block abi serialization off the main thread Feb 7, 2023
greg7mdp
greg7mdp reviewed Feb 7, 2023
View reviewed changes
libraries/chain/abi_serializer.cpp Outdated Show resolved Hide resolved
libraries/chain/abi_serializer.cpp Outdated Show resolved Hide resolved
greg7mdp
greg7mdp requested changes Feb 7, 2023
View reviewed changes
Copy link
Contributor

@greg7mdp greg7mdp left a comment •
edited
Loading

Choose a reason for hiding this comment

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

delaying approval because more strings should be moved (as indicated above).

@heifner heifner added the OCI Work exclusive to OCI team label Feb 7, 2023
@heifner heifner requested a review from linh2931 February 7, 2023 21:20
@heifner heifner mentioned this pull request Feb 7, 2023
Closed
greg7mdp
greg7mdp reviewed Feb 7, 2023
View reviewed changes
libraries/chain/abi_serializer.cpp Outdated Show resolved Hide resolved
unittests/abi_tests.cpp Outdated Show resolved Hide resolved
@heifner heifner mentioned this pull request Feb 8, 2023
Closed
@heifner heifner merged commit dd59a99 into main Feb 15, 2023
@heifner heifner deleted the GH-677-get-block branch February 15, 2023 13:24
This was referenced Feb 23, 2023
Closed
Merged
@stephenpdeos stephenpdeos mentioned this pull request Feb 23, 2023
Closed
@greg7mdp greg7mdp mentioned this pull request Apr 10, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@linh2931 linh2931 linh2931 approved these changes

@greg7mdp greg7mdp greg7mdp approved these changes

Assignees
No one assigned
Labels
OCI Work exclusive to OCI team
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

http requests fails with timeout on large block (json.hpp issue?)
3 participants
@heifner @greg7mdp @linh2931