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

1691 consider pvf executor params #2218

Merged
merged 9 commits into from
Sep 24, 2024
6 changes: 0 additions & 6 deletions core/application/app_configuration.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -257,12 +257,6 @@ namespace kagome::application {
*/
virtual bool usePvfSubprocess() const = 0;

/**
* Timeout for Pvf check calls execution.
* Effective only when usePvfSubprocess() == true.
*/
virtual std::chrono::milliseconds pvfSubprocessDeadline() const = 0;

/**
* Max PVF execution threads or processes.
*/
Expand Down
7 changes: 0 additions & 7 deletions core/application/impl/app_configuration_impl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -898,8 +898,6 @@ namespace kagome::application {
"Number of threads that precompile parachain runtime modules at node startup")
("parachain-single-process", po::bool_switch(),
"Disables spawn of child pvf check processes, thus they could not be aborted by deadline timer")
("parachain-check-deadline", po::value<uint32_t>()->default_value(2000),
"Pvf check subprocess execution deadline in milliseconds")
("pvf-max-workers", po::value<size_t>()->default_value(pvf_max_workers_),
"Max PVF execution threads or processes.")
("insecure-validator-i-know-what-i-do", po::bool_switch(), "Allows a validator to run insecurely outside of Secure Validator Mode.")
Expand Down Expand Up @@ -1520,11 +1518,6 @@ namespace kagome::application {
}
logger_->info("Parachain multi process: {}", use_pvf_subprocess_);

if (auto arg = find_argument<uint32_t>(vm, "parachain-check-deadline");
arg.has_value()) {
pvf_subprocess_deadline_ = std::chrono::milliseconds(*arg);
}

if (auto arg = find_argument<size_t>(vm, "pvf-max-workers")) {
pvf_max_workers_ = *arg;
}
Expand Down
4 changes: 0 additions & 4 deletions core/application/impl/app_configuration_impl.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -226,9 +226,6 @@ namespace kagome::application {
bool usePvfSubprocess() const override {
return use_pvf_subprocess_;
}
std::chrono::milliseconds pvfSubprocessDeadline() const override {
return pvf_subprocess_deadline_;
}
size_t pvfMaxWorkers() const override {
return pvf_max_workers_;
}
Expand Down Expand Up @@ -387,7 +384,6 @@ namespace kagome::application {
std::thread::hardware_concurrency() / 2;
bool should_precompile_parachain_modules_{true};
bool use_pvf_subprocess_{true};
std::chrono::milliseconds pvf_subprocess_deadline_{2000};
size_t pvf_max_workers_{
std::max<size_t>(std::thread::hardware_concurrency(), 1)};
bool disable_secure_mode_{false};
Expand Down
2 changes: 1 addition & 1 deletion core/application/modes/precompile_wasm.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ namespace kagome::application::mode {
auto code_hash = hasher_->blake2b_256(bytes);
OUTCOME_TRY(config,
parachain::sessionParams(*parachain_api_, block.hash));
OUTCOME_TRY(module_factory_->precompile(code_hash, bytes, config));
OUTCOME_TRY(module_factory_->precompile(code_hash, bytes, config.context_params));
}
return outcome::success();
}
Expand Down
2 changes: 1 addition & 1 deletion core/parachain/pvf/module_precompiler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ namespace kagome::parachain {
cores.pop_back();
}
auto res = self->precompileModulesForCore(
stats, last_finalized, executor_params, ParachainCore{core});
stats, last_finalized, executor_params.context_params, ParachainCore{core});
if (!res) {
using namespace std::string_literals;
auto id = get_para_id(core);
Expand Down
2 changes: 1 addition & 1 deletion core/parachain/pvf/precheck.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ namespace kagome::parachain {
auto &code_zstd = *code_zstd_res.value();
auto res = [&]() -> outcome::result<void> {
OUTCOME_TRY(config, sessionParams(*parachain_api_, block.hash));
OUTCOME_TRY(pvf_pool_->precompile(code_hash, code_zstd, config));
OUTCOME_TRY(pvf_pool_->precompile(code_hash, code_zstd, config.context_params));
return outcome::success();
}();
if (res) {
Expand Down
10 changes: 7 additions & 3 deletions core/parachain/pvf/pvf_impl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -320,14 +320,15 @@ namespace kagome::parachain {
WasmCb cb) const {
CB_TRY(auto executor_params,
sessionParams(*parachain_api_, receipt.descriptor.relay_parent));
const auto &context_params = executor_params.context_params;

constexpr auto name = "validate_block";
CB_TRYV(pvf_pool_->precompile(code_hash, code_zstd, executor_params));
CB_TRYV(pvf_pool_->precompile(code_hash, code_zstd, context_params));
if (not app_configuration_->usePvfSubprocess()) {
// Reusing instances for PVF calls doesn't work, runtime calls start to
// crash on access out of memory bounds
KAGOME_PROFILE_START_L(log_, single_process_runtime_instantitation);
auto module_opt = pvf_pool_->getModule(code_hash, executor_params);
auto module_opt = pvf_pool_->getModule(code_hash, context_params);
if (!module_opt) {
SL_ERROR(log_,
"Runtime module supposed to be precompiled for parachain ID "
Expand All @@ -344,7 +345,7 @@ namespace kagome::parachain {
return cb(executor_->call<ValidationResult>(ctx, name, params));
}
workers_->execute({
.code_path = pvf_pool_->getCachePath(code_hash, executor_params),
.code_path = pvf_pool_->getCachePath(code_hash, context_params),
.args = scale::encode(params).value(),
.cb =
[cb{std::move(cb)}](outcome::result<common::Buffer> r) {
Expand All @@ -353,6 +354,9 @@ namespace kagome::parachain {
}
cb(scale::decode<ValidationResult>(r.value()));
},
.timeout =
std::chrono::milliseconds{
executor_params.pvf_exec_timeout_backing_ms},
});
}

Expand Down
18 changes: 18 additions & 0 deletions core/parachain/pvf/runtime_params.hpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
#pragma once

#include "runtime/runtime_context.hpp"

namespace kagome::parachain {
static constexpr uint64_t DEFAULT_BACKING_EXECUTION_TIMEOUT = 2000;
static constexpr uint64_t DEFAULT_APPROVAL_EXECUTION_TIMEOUT = 12000;

struct RuntimeParams {
SCALE_TIE(3);
kagome::runtime::RuntimeContext::ContextParams context_params;
uint64_t pvf_exec_timeout_approval_ms = DEFAULT_APPROVAL_EXECUTION_TIMEOUT;
uint64_t pvf_exec_timeout_backing_ms = DEFAULT_BACKING_EXECUTION_TIMEOUT;
};

} // namespace kagome::parachain

SCALE_TIE_HASH_STD(kagome::parachain::RuntimeParams);
30 changes: 23 additions & 7 deletions core/parachain/pvf/session_params.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -9,27 +9,43 @@
#include "runtime/runtime_api/parachain_host.hpp"
#include "runtime/runtime_context.hpp"

#include "parachain/pvf/runtime_params.hpp"
namespace kagome::parachain {
inline outcome::result<runtime::RuntimeContext::ContextParams> sessionParams(
inline outcome::result<RuntimeParams> sessionParams(
runtime::ParachainHost &api, const primitives::BlockHash &relay_parent) {
// https://github.com/paritytech/polkadot-sdk/blob/e0c081dbd46c1e6edca1ce2c62298f5f3622afdd/polkadot/node/core/pvf/common/src/executor_interface.rs#L46-L47
constexpr uint32_t kDefaultHeapPagesEstimate = 32;
constexpr uint32_t kExtraHeapPages = 2048;
OUTCOME_TRY(session_index, api.session_index_for_child(relay_parent));
OUTCOME_TRY(session_params,
api.session_executor_params(relay_parent, session_index));
runtime::RuntimeContext::ContextParams config;
config.memory_limits.max_stack_values_num =
RuntimeParams config;
auto &contextParams = config.context_params;
contextParams.memory_limits.max_stack_values_num =
runtime::RuntimeContext::DEFAULT_STACK_MAX;
config.memory_limits.heap_alloc_strategy =
contextParams.memory_limits.heap_alloc_strategy =
HeapAllocStrategyDynamic{kDefaultHeapPagesEstimate + kExtraHeapPages};
if (session_params) {
for (auto &param : *session_params) {
if (auto *stack_max = get_if<runtime::StackLogicalMax>(&param)) {
config.memory_limits.max_stack_values_num = stack_max->max_values_num;
contextParams.memory_limits.max_stack_values_num =
stack_max->max_values_num;
} else if (auto *pages_max = get_if<runtime::MaxMemoryPages>(&param)) {
config.memory_limits.heap_alloc_strategy = HeapAllocStrategyDynamic{
kDefaultHeapPagesEstimate + pages_max->limit};
contextParams.memory_limits.heap_alloc_strategy =
HeapAllocStrategyDynamic{kDefaultHeapPagesEstimate
+ pages_max->limit};
} else if (auto *pvfExecTimeout =
get_if<runtime::PvfExecTimeout>(&param)) {
switch (pvfExecTimeout->kind) {
case runtime::PvfExecTimeoutKind::Backing:
config.pvf_exec_timeout_backing_ms = pvfExecTimeout->msec;
break;
case runtime::PvfExecTimeoutKind::Approval:
config.pvf_exec_timeout_approval_ms = pvfExecTimeout->msec;
break;
default:
break;
}
kamilsa marked this conversation as resolved.
Show resolved Hide resolved
}
}
}
Expand Down
3 changes: 1 addition & 2 deletions core/parachain/pvf/workers.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,6 @@ namespace kagome::parachain {
scheduler_{std::move(scheduler)},
exe_{exePath()},
max_{app_config.pvfMaxWorkers()},
timeout_{app_config.pvfSubprocessDeadline()},
Copy link
Contributor

Choose a reason for hiding this comment

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

If we are not using pvf subprocess deadline anymore it should be removed from CLI as well (app_configuration_impl)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

worker_config_{
.engine = pvf_runtime_engine(app_config),
.cache_dir = app_config.runtimeCacheDirPath(),
Expand Down Expand Up @@ -216,7 +215,7 @@ namespace kagome::parachain {
timeout->reset();
};
*timeout = scheduler_->scheduleWithHandle(
[cb]() mutable { cb(std::errc::timed_out); }, timeout_);
[cb]() mutable { cb(std::errc::timed_out); }, job.timeout);
worker.process->writeScale(PvfWorkerInput{job.args},
[cb](outcome::result<void> r) mutable {
if (not r) {
Expand Down
2 changes: 1 addition & 1 deletion core/parachain/pvf/workers.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ namespace kagome::parachain {
PvfWorkerInputCodePath code_path;
Buffer args;
Cb cb;
std::chrono::milliseconds timeout{0};
};
void execute(Job &&job);

Expand Down Expand Up @@ -73,7 +74,6 @@ namespace kagome::parachain {
std::shared_ptr<libp2p::basic::Scheduler> scheduler_;
std::filesystem::path exe_;
size_t max_;
std::chrono::milliseconds timeout_;
PvfWorkerInputConfig worker_config_;
std::list<Worker> free_;
size_t used_ = 0;
Expand Down
1 change: 0 additions & 1 deletion core/runtime/runtime_context.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@ namespace kagome::runtime {

// https://github.com/paritytech/polkadot-sdk/blob/e16ef0861f576dd260487d78b57949b18795ed77/polkadot/primitives/src/v6/executor_params.rs#L32
static constexpr size_t DEFAULT_STACK_MAX = 65536;

struct ContextParams {
SCALE_TIE(1);
MemoryLimits memory_limits;
Expand Down
5 changes: 0 additions & 5 deletions test/mock/core/application/app_configuration_mock.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -139,11 +139,6 @@ namespace kagome::application {

MOCK_METHOD(bool, usePvfSubprocess, (), (const, override));

MOCK_METHOD(std::chrono::milliseconds,
pvfSubprocessDeadline,
(),
(const, override));

MOCK_METHOD(size_t, pvfMaxWorkers, (), (const, override));

MOCK_METHOD(bool, disableSecureMode, (), (const, override));
Expand Down
Loading
Loading