-
Notifications
You must be signed in to change notification settings - Fork 90
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
add fork support in runtime light #1029
Conversation
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.
LGTM
58b5083
to
29c4d07
Compare
101e17e
to
245e44c
Compare
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.
LGTM!
timer_d = arg.timeout_d; | ||
} else if constexpr (std::is_same_v<T, stream_future>) { | ||
timer_d = arg.timeout_d; | ||
} else { |
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.
Let's also add static_assert(false, "non-exhaustive visitor!");
here
}; | ||
|
||
template<typename T> | ||
using internal_optional_type = typename InternalOptionalType<T>::type; |
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.
Let's name it internal_optional_type_t
in a manner that standard lib names such traits
0708bb6
to
2071f5e
Compare
2071f5e
to
614792f
Compare
|
||
static KphpForkContext &get(); | ||
|
||
KphpForkContext(memory_resource::unsynchronized_pool_resource &memory_pool) |
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.
KphpForkContext(memory_resource::unsynchronized_pool_resource &memory_pool) | |
explicit KphpForkContext(memory_resource::unsynchronized_pool_resource &memory_pool) |
running_forks[fork_id] = light_fork(std::move(task)); | ||
running_forks[fork_id].resume(fork_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.
maybe we'll get rid of double lookup?
template<typename T> | ||
using deque = memory_resource::stl::deque<T, memory_resource::unsynchronized_pool_resource>; | ||
|
||
fork_scheduler(memory_resource::unsynchronized_pool_resource &memory_pool) |
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.
fork_scheduler(memory_resource::unsynchronized_pool_resource &memory_pool) | |
explicit fork_scheduler(memory_resource::unsynchronized_pool_resource &memory_pool) |
and check out other similar places pls
if (func->has_global_vars_inside) { | ||
W << PhpMutableGlobalsAssignCurrent() << NL; | ||
} | ||
W << "vk::final_action action([]{KphpForkContext::get().scheduler.mark_current_fork_as_ready();});" << NL; |
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.
Let's recheck that it's working in case when current function is called from other fork. It's a bit strange that we change codegen of function we want to fork. It should be redundant when we use C++-20 coroutines.
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 think it's better to determine that fork is ready outside of its logic, i.e. somewhere in scheduler
if (func->kphp_tracing) { | ||
TracingAutogen::codegen_runtime_func_guard_declaration(W, func); | ||
TracingAutogen::codegen_runtime_func_guard_start(W, func); | ||
} | ||
compile_tracing_profiler(func, W); | ||
|
||
for (auto var : func->local_var_ids) { | ||
if (var->type() != VarData::var_local_inplace_t && !var->is_foreach_reference) { | ||
W << VarDeclaration(var); | ||
} | ||
} | ||
|
||
if (func->has_variadic_param) { | ||
auto params = func->get_params(); | ||
kphp_assert(!params.empty()); | ||
auto variadic_arg = std::prev(params.end()); | ||
auto name_of_variadic_param = VarName(variadic_arg->as<op_func_param>()->var()->var_id); | ||
W << "if (!" << name_of_variadic_param << ".is_vector())" << BEGIN; | ||
W << "php_warning(\"pass associative array(" << name_of_variadic_param << ") to variadic function: " << FunctionName(func) << "\");" << NL; | ||
W << name_of_variadic_param << " = f$array_values(" << name_of_variadic_param << ");" << NL; | ||
W << END << NL; | ||
} | ||
W << AsSeq{func_root->cmd()} << END << NL; |
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.
Also let's recheck that it's actually needed in our case
// Distributed under the GPL v3 License, see LICENSE.notice.txt | ||
|
||
#pragma once | ||
|
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.
let's add midding includes like coroutine
, type_traits
etc.
unordered_set<int64_t> ready_forks; | ||
/* map from runtime_future {stream_future, fork_future} to waiting fork*/ | ||
unordered_map<runtime_future, int64_t> future_to_blocked_fork; | ||
/* reverse of future_to_blocked_fork */ | ||
unordered_map<int64_t, runtime_future> fork_to_awaited_future; | ||
/* timeout timer_d to waiting fork */ | ||
unordered_map<uint64_t, int64_t> timer_to_blocked_fork; | ||
/* set of forks that wait to accept incoming stream */ | ||
unordered_set<int64_t> wait_incoming_query_forks; | ||
|
||
deque<int64_t> forks_ready_to_resume; |
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 suggest to add detailed comment about typical fork lifecycle and its transitions between these hash tables
Fork support in runtime light
This pr add
fork
andwait
functions in runtime light. The main difference from forks in the master is the use of C++20 coroutinesOveriew
The class that stores the current state of the forks is
KphpForkContext
. It containsfork_scheduler
, which is responsible for executing forks. In the schedule function it tries to execute all possible coroutines, if the main fork runs out or there are no forks available for execution the component is blocked.The fork stored in the
light_fork
class. It stores a pointer to the first coroutine frame and the last interrupt point. Thefork_result
type erasue class is used to store the result.