-
Notifications
You must be signed in to change notification settings - Fork 220
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
Multithreaded cryptography #850
Multithreaded cryptography #850
Conversation
this yields more efficient througput. probably.
@@ -39,7 +39,7 @@ struct DemoCall : public abyss::http::IRPCClientHandler | |||
bool HandleResponse(abyss::http::RPC_Response) override | |||
{ | |||
llarp::LogInfo("response get"); | |||
m_Logic->queue_func(m_Callback); | |||
m_Logic->queue_func([=]() { m_Callback(); }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't understand what this change achieves?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
storing a snapshot of the scope? I see this type of hack in JS a lot
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
js is pass by reference, C++ is pass by value (unless explicitly noted otherwise)
std::bit_xor< size_t >()), | ||
std::bit_xor< size_t >()); | ||
} | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you could use abseil hashing instead:
template <typename H>
H AbslHashValue(H h, const Path& p) {
return H::combine(std::move(h), p.hops[0].upstream, p.hops[0].rxID, p.hops[0].txID);
}
(providing the sub-types were also hashable)
llarp/util/thread/logic.cpp
Outdated
@@ -97,7 +109,7 @@ namespace llarp | |||
bool | |||
Logic::can_flush() const | |||
{ | |||
return ourID == std::this_thread::get_id(); | |||
return id.has_value() && id.value() == std::this_thread::get_id(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this can just be id == std::this_thread::get_id()
@@ -433,7 +433,8 @@ extern "C" | |||
void | |||
llarp_main_signal(struct llarp_main *ptr, int sig) | |||
{ | |||
ptr->ctx->HandleSignal(sig); | |||
ptr->ctx->logic->queue_func( | |||
std::bind(&llarp::Context::HandleSignal, ptr->ctx.get(), sig)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is probably clearer as:
ptr->ctx->logic->queue_func([ctx=ptr->ctx.get(),sig]() { ctx->HandleSignal(sig); });
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for that matter, as long as queue_func takes an r-value reference it could be even cleaner to do:
auto func = <make the lambda>; logic->queue_func(func);
as 2 separate lines, but either way it's a style consideration.
this makes most of the cryptography done in worker threads.
includes iwp multiack and public key pinning because separating them would effectively mean rewriting that part so eh why not include it. this DOES break the protocol handshake but we're going to do that anyways.