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

Parachain runtime instance cache #1687

Merged
merged 37 commits into from
Aug 21, 2023
Merged

Conversation

Harrm
Copy link
Contributor

@Harrm Harrm commented Jul 11, 2023

Referenced issues

closes #1670

Description of the Change

Adds a parachain runtime instance cache, so that parachains don't instantiate a WASM module every time they want to call a runtime method. Also, respect the memory page limit returned by session_executor_params runtime call.

Benefits

Faster PVF instantiation.

Possible Drawbacks

Increased memory consumption.

@Harrm Harrm requested review from turuslan and xDimon July 24, 2023 08:18
@Harrm Harrm marked this pull request as ready for review July 24, 2023 08:18
@codecov
Copy link

codecov bot commented Jul 24, 2023

Codecov Report

Merging #1687 (2cbe0a3) into master (0cd656a) will increase coverage by 1.79%.
The diff coverage is 37.75%.

❗ Current head 2cbe0a3 differs from pull request most recent head 3a1bbca. Consider uploading reports for the commit 3a1bbca to get more accurate results

@@            Coverage Diff             @@
##           master    #1687      +/-   ##
==========================================
+ Coverage   21.56%   23.36%   +1.79%     
==========================================
  Files         745      708      -37     
  Lines       31935    29284    -2651     
  Branches    16586    15090    -1496     
==========================================
- Hits         6887     6841      -46     
+ Misses      19286    16722    -2564     
+ Partials     5762     5721      -41     
Files Changed Coverage Δ
core/api/service/system/impl/system_api_impl.cpp 39.02% <0.00%> (ø)
core/application/app_configuration.hpp 100.00% <ø> (ø)
core/application/impl/app_configuration_impl.hpp 23.86% <0.00%> (ø)
core/authorship/impl/block_builder_impl.hpp 100.00% <ø> (ø)
core/benchmark/block_execution_benchmark.cpp 0.00% <ø> (ø)
core/consensus/grandpa/impl/environment_impl.cpp 12.03% <0.00%> (-5.34%) ⬇️
core/injector/calculate_genesis_state.hpp 31.81% <0.00%> (+1.38%) ⬆️
core/network/impl/protocols/light.hpp 0.00% <ø> (ø)
core/network/impl/state_sync_request_flow.cpp 0.00% <0.00%> (ø)
core/network/types/collator_messages.hpp 15.38% <0.00%> (+15.38%) ⬆️
... and 62 more

... and 110 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

core/runtime/runtime_api/parachain_host_types.hpp Outdated Show resolved Hide resolved
core/runtime/common/memory_allocator.hpp Outdated Show resolved Hide resolved
core/injector/calculate_genesis_state.hpp Outdated Show resolved Hide resolved
core/runtime/common/executor.cpp Outdated Show resolved Hide resolved
core/runtime/common/executor.cpp Outdated Show resolved Hide resolved
logger_->error(
"Memory size exceeded when growing it on {} bytes, offset was 0x{:x}",
chunk_sz,
offset_);
return 0;
}
resize(offset_ + chunk_sz);
auto new_size = offset_ + chunk_sz;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
auto new_size = offset_ + chunk_sz;
auto new_size = new_pages_num * kMemoryPageSize;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What for? resize() aligns the new size anyway.

Comment on lines 31 to 58
class RuntimeContext {
public:
// should be created from runtime contex factory
RuntimeContext() = delete;
RuntimeContext(const RuntimeContext &) = delete;
RuntimeContext &operator=(const RuntimeContext &) = delete;

RuntimeContext(RuntimeContext &&) = default;

// constructor for tests
static RuntimeContext create_TEST(
std::shared_ptr<ModuleInstance> module_instance) {
return RuntimeContext{module_instance};
}

struct ContextParams {
ContextParams() = delete;

MemoryLimits memory_limits;
};

const std::shared_ptr<ModuleInstance> module_instance;

private:
friend class RuntimeContextFactoryImpl;
friend class RuntimeContextFactory;
RuntimeContext(std::shared_ptr<ModuleInstance> module_instance);
};
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 use ModuleInstance directly?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, this should be noted in comments. ModuleInstances are reused, but before every call there should be some setup. RuntimeContext enforces this setup. Resetting memory, setting the correct batch.

core/runtime/runtime_context.hpp Show resolved Hide resolved
Comment on lines 37 to 39
virtual outcome::result<Buffer> callWithCtx(RuntimeContext &ctx,
std::string_view name,
const Buffer &encoded_args);
Copy link
Contributor

Choose a reason for hiding this comment

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

Move this wrapper method to ModuleInstance

@igor-egorov igor-egorov requested review from igor-egorov and removed request for xDimon July 27, 2023 08:43
core/parachain/pvf/pvf_impl.cpp Outdated Show resolved Hide resolved
core/parachain/pvf/pvf_impl.hpp Outdated Show resolved Hide resolved
core/parachain/pvf/pvf_impl.hpp Outdated Show resolved Hide resolved
core/parachain/pvf/pvf_impl.cpp Outdated Show resolved Hide resolved
Comment on lines 191 to 193
OUTCOME_TRY(
parent_hash,
block_header_repository_->getHashByNumber(params.relay_parent_number));
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
OUTCOME_TRY(
parent_hash,
block_header_repository_->getHashByNumber(params.relay_parent_number));

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Why?

Copy link
Contributor

Choose a reason for hiding this comment

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

Not used in rust, also num -> hash is ambigous

core/parachain/pvf/pvf_impl.cpp Outdated Show resolved Hide resolved
Comment on lines 51 to 56
std::unordered_map<ParachainId, Entry> instance_cache_;
std::map<uint64_t, ParachainId> last_usage_time_;
const uint32_t instances_limit_ = 43;
std::atomic<uint64_t> time_ = 0;

void erase(decltype(instance_cache_)::iterator it);
Copy link
Contributor

Choose a reason for hiding this comment

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

  • Lru.
  • Keep 2 codes for para near upgrade, also there are less codes then paras.
Suggested change
std::unordered_map<ParachainId, Entry> instance_cache_;
std::map<uint64_t, ParachainId> last_usage_time_;
const uint32_t instances_limit_ = 43;
std::atomic<uint64_t> time_ = 0;
void erase(decltype(instance_cache_)::iterator it);
Lru<ValidationCodeHash, Entry> instance_cache_;

core/parachain/pvf/pvf_runtime_cache.hpp Outdated Show resolved Hide resolved
Comment on lines 27 to 28
using SafeInstance = SafeObject<std::shared_ptr<runtime::ModuleInstance>>;
using SafeInstanceRef = std::reference_wrapper<SafeInstance>;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
using SafeInstance = SafeObject<std::shared_ptr<runtime::ModuleInstance>>;
using SafeInstanceRef = std::reference_wrapper<SafeInstance>;

Comment on lines 33 to 36
outcome::result<SafeInstanceRef> requestInstance(
ParachainId para_id,
const common::Hash256 &code_hash,
const ParachainRuntime &code_zstd);
Copy link
Contributor

Choose a reason for hiding this comment

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

  • ~BorrowedInstance() will call pool(self.borrowed)
Suggested change
outcome::result<SafeInstanceRef> requestInstance(
ParachainId para_id,
const common::Hash256 &code_hash,
const ParachainRuntime &code_zstd);
outcome::result<std::shared_ptr<runtime::ModuleInstance>> requestInstance(
const common::Hash256 &code_hash,
const ParachainRuntime &code_zstd);
pool(std::shared_ptr<runtime::ModuleInstance>);

Comment on lines 24 to 91
PvfRuntimeCache::requestInstance(ParachainId para_id,
const common::Hash256 &code_hash,
const ParachainRuntime &code_zstd) {
++time_;
std::unique_lock lock{instance_cache_mutex_};
auto it = instance_cache_.find(para_id);

bool it_found = it != instance_cache_.end();

bool same_hash =
it_found && it->second.instance.sharedAccess([](const auto &instance) {
return instance->getCodeHash();
}) == code_hash;

if (!(it_found && same_hash)) {
ParachainRuntime code;
OUTCOME_TRY(runtime::uncompressCodeIfNeeded(code_zstd, code));
OUTCOME_TRY(runtime_module, module_factory_->make(code));
OUTCOME_TRY(instance, runtime_module->instantiate());
if (it_found) {
erase(it);
}
SafeObject safe_instance{instance};

auto [new_it, inserted] =
instance_cache_.emplace(std::piecewise_construct,
std::forward_as_tuple(para_id),
std::forward_as_tuple(instance, time_));
BOOST_ASSERT(inserted);
last_usage_time_.emplace(time_, para_id);
it = new_it;
if (instance_cache_.size() > instances_limit_) {
cleanup(lock);
}
} else {
auto lru_it = last_usage_time_.find(it->second.last_used);
BOOST_ASSERT(lru_it != last_usage_time_.end());
last_usage_time_.erase(lru_it);
last_usage_time_.emplace(time_, para_id);
it->second.last_used = time_;
}
BOOST_ASSERT(it != instance_cache_.end());
return it->second.instance;
}

void PvfRuntimeCache::cleanup(const std::unique_lock<std::mutex> &lock) {
BOOST_ASSERT(lock.owns_lock());
BOOST_ASSERT(lock.mutex() == &instance_cache_mutex_);

for (auto it = last_usage_time_.begin();
it != last_usage_time_.end()
&& last_usage_time_.size() > instances_limit_;) {
instance_cache_.erase(it->second);
it = last_usage_time_.erase(it);
}
}

void PvfRuntimeCache::erase(decltype(instance_cache_)::iterator it) {
// instance can be used at this point, so we wait for exclusive access
// and then extract it from the map (we cannot erase it from inside
// the exclusive access callback because destroying a locked mutex is
// UB)
it->second.instance.exclusiveAccess(
[this, it](auto) {
last_usage_time_.erase(it->second.last_used);
return instance_cache_.extract(it);
});
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
PvfRuntimeCache::requestInstance(ParachainId para_id,
const common::Hash256 &code_hash,
const ParachainRuntime &code_zstd) {
++time_;
std::unique_lock lock{instance_cache_mutex_};
auto it = instance_cache_.find(para_id);
bool it_found = it != instance_cache_.end();
bool same_hash =
it_found && it->second.instance.sharedAccess([](const auto &instance) {
return instance->getCodeHash();
}) == code_hash;
if (!(it_found && same_hash)) {
ParachainRuntime code;
OUTCOME_TRY(runtime::uncompressCodeIfNeeded(code_zstd, code));
OUTCOME_TRY(runtime_module, module_factory_->make(code));
OUTCOME_TRY(instance, runtime_module->instantiate());
if (it_found) {
erase(it);
}
SafeObject safe_instance{instance};
auto [new_it, inserted] =
instance_cache_.emplace(std::piecewise_construct,
std::forward_as_tuple(para_id),
std::forward_as_tuple(instance, time_));
BOOST_ASSERT(inserted);
last_usage_time_.emplace(time_, para_id);
it = new_it;
if (instance_cache_.size() > instances_limit_) {
cleanup(lock);
}
} else {
auto lru_it = last_usage_time_.find(it->second.last_used);
BOOST_ASSERT(lru_it != last_usage_time_.end());
last_usage_time_.erase(lru_it);
last_usage_time_.emplace(time_, para_id);
it->second.last_used = time_;
}
BOOST_ASSERT(it != instance_cache_.end());
return it->second.instance;
}
void PvfRuntimeCache::cleanup(const std::unique_lock<std::mutex> &lock) {
BOOST_ASSERT(lock.owns_lock());
BOOST_ASSERT(lock.mutex() == &instance_cache_mutex_);
for (auto it = last_usage_time_.begin();
it != last_usage_time_.end()
&& last_usage_time_.size() > instances_limit_;) {
instance_cache_.erase(it->second);
it = last_usage_time_.erase(it);
}
}
void PvfRuntimeCache::erase(decltype(instance_cache_)::iterator it) {
// instance can be used at this point, so we wait for exclusive access
// and then extract it from the map (we cannot erase it from inside
// the exclusive access callback because destroying a locked mutex is
// UB)
it->second.instance.exclusiveAccess(
[this, it](auto) {
last_usage_time_.erase(it->second.last_used);
return instance_cache_.extract(it);
});
}
PvfRuntimeCache::requestInstance(const common::Hash256 &code_hash,
const ParachainRuntime &code_zstd) {
std::unique_lock lock{instance_cache_mutex_};
std::shared_ptr<runtime::ModuleInstance> instance;
if (auto entry = instance_cache_.get(code_hash)) {
if (entry.instances.empty()) {
BOOST_OUTCOME_TRY(instance, entry.module->instantiate());
} else {
instance = entry->instances.back();
entry->instances.pop_back();
}
} else {
ParachainRuntime code;
OUTCOME_TRY(runtime::uncompressCodeIfNeeded(code_zstd, code));
OUTCOME_TRY(runtime_module, module_factory_->make(code));
BOOST_OUTCOME_TRY(instance, runtime_module->instantiate());
cache_.put(code_hash, {module, {}});
}
return std::make_shared<runtime::BorrowedInstance>(weak_from_this(), instance);
}
void PvfRuntimeCache::pool(std::shared_ptr<runtime::ModuleInstance> instance) {
std::unique_lock lock{instance_cache_mutex_};
auto module = instance->getModule();
auto hash = module->getHash();
if (auto entry = instance_cache_.get(hash)) {
entry->instances.emplace_back(instance);
} else {
instance_cache_.put(hash, {module, {instance}});
}
}

Comment on lines 28 to 36
outcome::result<primitives::Version> CoreImpl::version(
RuntimeEnvironment &env) {
return executor_->call<primitives::Version>(env, "Core_version");
std::shared_ptr<ModuleInstance> instance) {
OUTCOME_TRY(genesis_hash, header_repo_->getHashByNumber(0));
OUTCOME_TRY(genesis_header, header_repo_->getBlockHeader(genesis_hash));
OUTCOME_TRY(ctx,
ctx_factory_->ephemeral(instance, genesis_header.state_root));
return executor_->decodedCallWithCtx<primitives::Version>(ctx,
"Core_version");
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove this method, if only used by calculate_genesis_state, or remove ambigous argument

class PvfRuntimeCache {
public:
using SafeInstance = SafeObject<std::shared_ptr<runtime::ModuleInstance>>;
using SafeInstanceRef = std::reference_wrapper<SafeInstance>;
Copy link
Contributor

Choose a reason for hiding this comment

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

SafeInstanceRef will become invalid when lru evicts it

@Harrm Harrm merged commit 153a078 into master Aug 21, 2023
@Harrm Harrm deleted the feature/parachain-runtime-instance-cache branch August 21, 2023 14:23
Harrm added a commit that referenced this pull request Aug 28, 2023
* Add cache for runtime instances in pvf_impl

* Refactor runtime environment factory

* Executor refactoring for testability 

Co-authored-by: Ruslan Tushov <[email protected]>
Co-authored-by: Ruslan Tushov <[email protected]>
Harrm added a commit that referenced this pull request Sep 1, 2023
* Add cache for runtime instances in pvf_impl

* Refactor runtime environment factory

* Executor refactoring for testability

Co-authored-by: Ruslan Tushov <[email protected]>
Co-authored-by: Ruslan Tushov <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Runtime module cache
3 participants