From e8f2b788d4e7146c5320d5d647fb43007822d0d8 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Wed, 23 Apr 2014 09:01:57 -0700 Subject: [PATCH] Reduce allocations when computing rule transitions --- .../build_tables/merge_transitions_spec.cc | 19 ++++++----- .../build_tables/item_set_transitions.cc | 12 +++---- src/compiler/build_tables/merge_transitions.h | 32 ++++++++----------- src/compiler/build_tables/rule_transitions.cc | 22 ++++++------- 4 files changed, 39 insertions(+), 46 deletions(-) diff --git a/spec/compiler/build_tables/merge_transitions_spec.cc b/spec/compiler/build_tables/merge_transitions_spec.cc index 75434c84..d8686e11 100644 --- a/spec/compiler/build_tables/merge_transitions_spec.cc +++ b/spec/compiler/build_tables/merge_transitions_spec.cc @@ -9,8 +9,11 @@ START_TEST describe("merging character set transitions", []() { typedef map int_map; - auto bitwise = [](int l, int r) -> int { - return l | r; + auto merge_result = [&](int_map left, int_map right) -> int_map { + merge_char_transitions(left, right, [](int l, int r) -> int { + return l | r; + }); + return left; }; describe("when none of the transitions intersect", [&]() { @@ -26,7 +29,7 @@ describe("merging character set transitions", []() { { CharacterSet({ '\t' }), 16 }, }); - AssertThat(merge_char_transitions(map1, map2, bitwise), Equals(int_map({ + AssertThat(merge_result(map1, map2), Equals(int_map({ { CharacterSet({ 'a', 'c' }), 1 }, { CharacterSet({ 'x', 'y' }), 2 }, { CharacterSet({ '1', '9' }), 4 }, @@ -34,7 +37,7 @@ describe("merging character set transitions", []() { { CharacterSet({ '\t' }), 16 }, }))); - AssertThat(merge_char_transitions(map2, map1, bitwise), Equals(merge_char_transitions(map1, map2, bitwise))); + AssertThat(merge_result(map2, map1), Equals(merge_result(map1, map2))); }); }); @@ -50,14 +53,14 @@ describe("merging character set transitions", []() { { CharacterSet({ '3' }), 8 }, }); - AssertThat(merge_char_transitions(map1, map2, bitwise), Equals(int_map({ + AssertThat(merge_result(map1, map2), Equals(int_map({ { CharacterSet({ {'a', 'b'}, {'d', 'f'}, {'A', 'F'} }), 1 }, { CharacterSet({ {'c'} }), 5 }, { CharacterSet({ {'0', '2'}, {'4', '9'} }), 2 }, { CharacterSet({ '3' }), 10 }, }))); - AssertThat(merge_char_transitions(map2, map1, bitwise), Equals(merge_char_transitions(map1, map2, bitwise))); + AssertThat(merge_result(map2, map1), Equals(merge_result(map1, map2))); }); }); @@ -72,12 +75,12 @@ describe("merging character set transitions", []() { { CharacterSet({ 'c' }), 4 }, }); - AssertThat(merge_char_transitions(map1, map2, bitwise), Equals(int_map({ + AssertThat(merge_result(map1, map2), Equals(int_map({ { CharacterSet({ 'a' }), 3 }, { CharacterSet({ 'c' }), 5 }, }))); - AssertThat(merge_char_transitions(map2, map1, bitwise), Equals(merge_char_transitions(map1, map2, bitwise))); + AssertThat(merge_result(map2, map1), Equals(merge_result(map1, map2))); }); }); }); diff --git a/src/compiler/build_tables/item_set_transitions.cc b/src/compiler/build_tables/item_set_transitions.cc index 2f611443..4263855d 100644 --- a/src/compiler/build_tables/item_set_transitions.cc +++ b/src/compiler/build_tables/item_set_transitions.cc @@ -45,9 +45,9 @@ namespace tree_sitter { map result; for (const LexItem &item : item_set) { map item_transitions = char_transitions(item); - result = merge_char_transitions(result, - item_transitions, - [](LexItemSet left, LexItemSet right) { + merge_char_transitions(result, + item_transitions, + [](LexItemSet left, LexItemSet right) { return merge_sets(left, right); }); } @@ -59,9 +59,9 @@ namespace tree_sitter { map result; for (const ParseItem &item : item_set) { map item_transitions = sym_transitions(item, grammar); - result = merge_sym_transitions(result, - item_transitions, - [&](ParseItemSet left, ParseItemSet right) { + merge_sym_transitions(result, + item_transitions, + [&](ParseItemSet left, ParseItemSet right) { return merge_sets(left, right); }); } diff --git a/src/compiler/build_tables/merge_transitions.h b/src/compiler/build_tables/merge_transitions.h index 223c385b..f5127643 100644 --- a/src/compiler/build_tables/merge_transitions.h +++ b/src/compiler/build_tables/merge_transitions.h @@ -15,15 +15,13 @@ namespace tree_sitter { * using the given function. */ template - std::map - merge_sym_transitions(const std::map &left, - const std::map &right, - std::function merge_fn) { - std::map result(left); + void merge_sym_transitions(std::map &left, + const std::map &right, + std::function merge_fn) { for (auto &pair : right) { auto rule = pair.first; bool merged = false; - for (auto &existing_pair : result) { + for (auto &existing_pair : left) { auto existing_rule = existing_pair.first; if (existing_rule == rule) { existing_pair.second = merge_fn(existing_pair.second, pair.second); @@ -32,9 +30,8 @@ namespace tree_sitter { } } if (!merged) - result.insert({ pair.first, pair.second }); + left.insert({ pair.first, pair.second }); } - return result; } /* @@ -44,19 +41,17 @@ namespace tree_sitter { * merging the two previous values using the given function. */ template - std::map - merge_char_transitions(const std::map &left, - const std::map &right, - std::function merge_fn) { - std::map result(left); + void merge_char_transitions(std::map &left, + const std::map &right, + std::function merge_fn) { for (auto &new_pair : right) { rules::CharacterSet new_char_set = new_pair.first; T new_value = new_pair.second; std::map pairs_to_insert; - auto iter = result.begin(); - while (iter != result.end()) { + auto iter = left.begin(); + while (iter != left.end()) { rules::CharacterSet char_set = iter->first; T value = iter->second; @@ -66,18 +61,17 @@ namespace tree_sitter { if (!char_set.is_empty()) pairs_to_insert.insert({ char_set, value }); pairs_to_insert.insert({ intersection, merge_fn(value, new_value) }); - result.erase(iter++); + left.erase(iter++); } else { ++iter; } } - result.insert(pairs_to_insert.begin(), pairs_to_insert.end()); + left.insert(pairs_to_insert.begin(), pairs_to_insert.end()); if (!new_char_set.is_empty()) - result.insert({ new_char_set, new_pair.second }); + left.insert({ new_char_set, new_pair.second }); } - return result; } } } diff --git a/src/compiler/build_tables/rule_transitions.cc b/src/compiler/build_tables/rule_transitions.cc index afa790a2..200ab508 100644 --- a/src/compiler/build_tables/rule_transitions.cc +++ b/src/compiler/build_tables/rule_transitions.cc @@ -24,21 +24,18 @@ namespace tree_sitter { namespace build_tables { template - map - merge_transitions(const map &left, const map &right); + void merge_transitions(map &left, const map &right); template<> - map - merge_transitions(const map &left, const map &right) { - return merge_char_transitions(left, right, [](rule_ptr left, rule_ptr right) { + void merge_transitions(map &left, const map &right) { + merge_char_transitions(left, right, [](rule_ptr left, rule_ptr right) { return make_shared(left, right); }); } template<> - map - merge_transitions(const map &left, const map &right) { - return merge_sym_transitions(left, right, [](rule_ptr left, rule_ptr right) { + void merge_transitions(map &left, const map &right) { + merge_sym_transitions(left, right, [](rule_ptr left, rule_ptr right) { return make_shared(left, right); }); } @@ -71,9 +68,9 @@ namespace tree_sitter { } map apply_to(const rules::Choice *rule) { - auto left_transitions = this->apply(rule->left); - auto right_transitions = this->apply(rule->right); - return merge_transitions(left_transitions, right_transitions); + auto result = this->apply(rule->left); + merge_transitions(result, this->apply(rule->right)); + return result; } map apply_to(const rules::Seq *rule) { @@ -81,8 +78,7 @@ namespace tree_sitter { return rules::Seq::Build({ left_rule, rule->right }); }); if (rule_can_be_blank(rule->left)) { - auto right_transitions = this->apply(rule->right); - result = merge_transitions(result, right_transitions); + merge_transitions(result, this->apply(rule->right)); } return result; }