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

Skip secondary indexes during replay #1918

Merged
merged 5 commits into from
Sep 13, 2019

Conversation

pmconrad
Copy link
Contributor

Fixes #452
Fixes #683

The indexes handled here are not used by consensus code. There's no need to update them during replay.
Two indexes have been moved into api_helper_indexes plugin.
The grouped_orders plugin has been modified to add its secondary index after replay, in the same way that api_helper_indexes does it.

I believe the risk I mentioned in #683 (comment) is sufficiently low.

Haven't measured speedup yet.

@pmconrad
Copy link
Contributor Author

Measured a speedup of 2% for replaying 40M blocks.

@pmconrad pmconrad force-pushed the 452-683-secondary-indexes branch from b45a4b2 to 8d263e1 Compare September 11, 2019 18:58
@pmconrad
Copy link
Contributor Author

Rebased to resolve conflict

Copy link
Member

@abitmore abitmore left a comment

Choose a reason for hiding this comment

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

Good job. Thanks.

libraries/plugins/grouped_orders/grouped_orders_plugin.cpp Outdated Show resolved Hide resolved
auto current_test_suite_id = boost::unit_test::framework::current_test_case().p_parent_id;
const auto current_test_name = boost::unit_test::framework::current_test_case().p_name.value;
const auto current_test_suite_id = boost::unit_test::framework::current_test_case().p_parent_id;
const auto current_suite_name = boost::unit_test::framework::get<boost::unit_test::test_suite>(current_test_suite_id)
Copy link
Member

Choose a reason for hiding this comment

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

Unfortunately this line is still too long, ideally at maximum 117 characters per line.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hm, now I see only 109 chars in the github diff view. ISTR that I used to see more.
Also it seems to depend on font size now (smaller font means more chars), but I'm sure I tried that before too and it didn't have an effect.

Copy link
Member

@abitmore abitmore Sep 13, 2019

Choose a reason for hiding this comment

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

Github is great and is always improving. There is a feature request: https://github.community/t5/How-to-use-Git-and-GitHub/Feature-Request-Increase-Unified-Diff-Width-to-120-Characters/m-p/30157

BTW there are tools to change the width in individual browsers, e.g. Wide Github for Chrome, Stylish for Firefox and Fathub for running in web dev console.

@@ -59,6 +59,8 @@ namespace graphene { namespace chain {
using std::cout;
using std::cerr;

namespace buf = boost::unit_test::framework;
Copy link
Member

Choose a reason for hiding this comment

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

buf is a commonly used abbreviation of buffer, which usually appears as a local variable or a function argument. I hope we don't use it in tests.

@abitmore abitmore mentioned this pull request Apr 16, 2020
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.

2 participants