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

Feature: core version and metadata cache #1391

Merged
merged 13 commits into from
Nov 11, 2022
Merged

Conversation

xDimon
Copy link
Member

@xDimon xDimon commented Nov 2, 2022

Referenced issues

Resolves #1374

Description of the Change

Cache by code hash for calls "Core_version" and "Metadata_metadata"

Benefits

Avoid runtime using for some idempotent calls

Usage Examples or Tests

Existing tests adapted

@xDimon xDimon force-pushed the feature/core_version_cache branch 2 times, most recently from 5bda630 to 2958405 Compare November 2, 2022 12:06
@codecov
Copy link

codecov bot commented Nov 2, 2022

Codecov Report

Merging #1391 (14191c1) into master (79ae107) will increase coverage by 0.00%.
The diff coverage is 30.09%.

@@           Coverage Diff           @@
##           master    #1391   +/-   ##
=======================================
  Coverage   24.44%   24.45%           
=======================================
  Files         633      638    +5     
  Lines       24035    24084   +49     
  Branches    12470    12492   +22     
=======================================
+ Hits         5875     5889   +14     
- Misses      12904    12938   +34     
- Partials     5256     5257    +1     
Impacted Files Coverage Δ
core/api/service/impl/api_service_impl.hpp 23.80% <ø> (ø)
core/blockchain/block_tree.hpp 100.00% <ø> (ø)
core/blockchain/impl/block_tree_impl.hpp 100.00% <ø> (+66.66%) ⬆️
core/runtime/binaryen/module/module_impl.hpp 100.00% <ø> (ø)
core/runtime/module_instance.hpp 100.00% <ø> (ø)
core/runtime/runtime_api/impl/core.cpp 20.83% <ø> (ø)
...runtime_api/impl/runtime_properties_cache_impl.cpp 0.00% <0.00%> (ø)
core/runtime/wavm/core_api_factory_impl.cpp 0.00% <0.00%> (ø)
core/runtime/wavm/instance_environment_factory.cpp 0.00% <0.00%> (ø)
core/runtime/wavm/module.cpp 0.00% <0.00%> (ø)
... and 17 more

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

@xDimon xDimon force-pushed the feature/core_version_cache branch 4 times, most recently from e7c760e to 09fe339 Compare November 8, 2022 20:36
Signed-off-by: Dmitriy Khaustov aka xDimon <[email protected]>
Signed-off-by: Dmitriy Khaustov aka xDimon <[email protected]>
Signed-off-by: Dmitriy Khaustov aka xDimon <[email protected]>
Signed-off-by: Dmitriy Khaustov aka xDimon <[email protected]>
@xDimon xDimon force-pushed the feature/core_version_cache branch from 09fe339 to efb26c8 Compare November 10, 2022 08:43
@xDimon xDimon marked this pull request as ready for review November 10, 2022 08:44
Copy link
Contributor

@igor-egorov igor-egorov left a comment

Choose a reason for hiding this comment

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

👍

external project test has failed on CI

Signed-off-by: Dmitriy Khaustov aka xDimon <[email protected]>
Signed-off-by: Dmitriy Khaustov aka xDimon <[email protected]>
Signed-off-by: Dmitriy Khaustov aka xDimon <[email protected]>
Signed-off-by: Dmitriy Khaustov aka xDimon <[email protected]>
Signed-off-by: Dmitriy Khaustov aka xDimon <[email protected]>
Signed-off-by: Dmitriy Khaustov aka xDimon <[email protected]>
Signed-off-by: Dmitriy Khaustov aka xDimon <[email protected]>
@xDimon xDimon enabled auto-merge (squash) November 11, 2022 21:50
@xDimon xDimon merged commit ea54fae into master Nov 11, 2022
@xDimon xDimon deleted the feature/core_version_cache branch November 11, 2022 22:44
: env_{std::move(env)},
rei_{std::move(rei)},
parent_{std::move(parent)},
code_hash_(code_hash),
Copy link
Contributor

Choose a reason for hiding this comment

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

hm, is this new field is even used in this class?

RuntimePropertiesCacheImpl() = default;

outcome::result<primitives::Version> getVersion(
const common::Hash256 &hash,
Copy link
Contributor

Choose a reason for hiding this comment

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

it could be either hash of block or hash of state, please give this variable more meaningful name

override;

outcome::result<primitives::OpaqueMetadata> getMetadata(
const common::Hash256 &hash,
Copy link
Contributor

Choose a reason for hiding this comment

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

same

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.

Cache for RuntiveVersion
4 participants