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

Reduce the number of thread hops for read-only trxs #1702

Merged
merged 6 commits into from
Oct 4, 2023

Conversation

heifner
Copy link
Member

@heifner heifner commented Oct 2, 2023

Optimize read-only trx execution by reducing the number of thread hops for execution.

This branch: https://github.com/AntelopeIO/leap/actions/runs/6365190135 16501 TPS
Compared with GH-1662-close: https://github.com/AntelopeIO/leap/actions/runs/6356928532 15001 TPS

@heifner heifner added this to the 1662 milestone Oct 2, 2023
@heifner heifner requested review from greg7mdp and linh2931 October 2, 2023 12:41
@heifner heifner added the OCI Work exclusive to OCI team label Oct 2, 2023
Base automatically changed from GH-1662-close to main October 3, 2023 15:52
// not thread safe, but as long as set_main_thread_id() only called at program startup from main thread before
// other threads are created then this will be safe to access.
std::thread::id get_main_thread_id() const {
assert(main_thread_id_ != std::thread::id());
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not initialize this in the member, so it is initialized when the executor is constructed?
std::thread::id main_thread_id_ { std::thread::id()) };

Copy link
Member Author

Choose a reason for hiding this comment

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

I was not thinking it would always be created on the main thread. But in practice I guess it would be.

Copy link
Member Author

Choose a reason for hiding this comment

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

Actually the tests create it on a diff thread. I can re-work those tests.

Copy link
Contributor

Choose a reason for hiding this comment

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

I assumed the Application was always created on the main thread, but if that's not the case you can ignore my comment.

Copy link
Member Author

Choose a reason for hiding this comment

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

In the nomial use-case it is, it just isn't for our tests, but I should be able to fix the tests.

@@ -164,8 +164,8 @@ inline auto make_http_response_handler(http_plugin_state& plugin_state, detail::
plugin_state.bytes_in_flight += payload_size;

// post back to an HTTP thread to allow the response handler to be called from any thread
boost::asio::post(plugin_state.thread_pool.get_executor(),
[&plugin_state, session_ptr, code, payload_size, response = std::move(response), content_type]() {
boost::asio::dispatch(plugin_state.thread_pool.get_executor(),
Copy link
Contributor

@greg7mdp greg7mdp Oct 3, 2023

Choose a reason for hiding this comment

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

I'm having a hard time figuring out why dispatch is appropriate here? Why can't we just execute the lambda here?

Copy link
Member Author

Choose a reason for hiding this comment

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

dispatch is the one that allows execution of the lambda here. post is the one that make sure it always executes outside this thread.

Copy link
Contributor

Choose a reason for hiding this comment

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

Do we really need to call dispatch instead of just executing the lambda's code here?

Copy link
Member Author

Choose a reason for hiding this comment

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

This can be called from the application thread, in that case we want to do the json parsing on the http thread.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
OCI Work exclusive to OCI team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants