From 3b388d66cdb4f30e10e28e7af3e7fb72ef2b4e4a Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Wed, 23 Apr 2014 08:32:11 -0700 Subject: [PATCH] Profile and optimize - Eliminate unnecessary copies of grammar objects - Do cheaper comparisons first in equality methods --- src/compiler/build_tables/first_set.cc | 10 +++++----- src/compiler/build_tables/lex_item.cc | 4 +--- src/compiler/build_tables/parse_item.cc | 10 +++++----- src/compiler/build_tables/rule_can_be_blank.cc | 8 ++++---- src/compiler/prepared_grammar.cc | 4 ++-- src/compiler/prepared_grammar.h | 4 ++-- src/compiler/rules/interned_symbol.cc | 2 +- 7 files changed, 20 insertions(+), 22 deletions(-) diff --git a/src/compiler/build_tables/first_set.cc b/src/compiler/build_tables/first_set.cc index da4ac721..216edb31 100644 --- a/src/compiler/build_tables/first_set.cc +++ b/src/compiler/build_tables/first_set.cc @@ -20,10 +20,10 @@ namespace tree_sitter { } class FirstSet : public rules::RuleFn> { - const PreparedGrammar grammar; + const PreparedGrammar *grammar; set visited_symbols; public: - explicit FirstSet(const PreparedGrammar &grammar) : grammar(grammar) {} + explicit FirstSet(const PreparedGrammar *grammar) : grammar(grammar) {} set apply_to(const ISymbol *rule) { if (visited_symbols.find(*rule) == visited_symbols.end()) { @@ -32,7 +32,7 @@ namespace tree_sitter { if (rule->is_token()) { return set({ *rule }); } else { - return apply(grammar.rule(*rule)); + return apply(grammar->rule(*rule)); } } else { return set(); @@ -49,7 +49,7 @@ namespace tree_sitter { set apply_to(const rules::Seq *rule) { auto result = apply(rule->left); - if (rule_can_be_blank(rule->left, grammar)) { + if (rule_can_be_blank(rule->left, *grammar)) { return set_union(result, apply(rule->right)); } else { return result; @@ -58,7 +58,7 @@ namespace tree_sitter { }; set first_set(const rules::rule_ptr &rule, const PreparedGrammar &grammar) { - return FirstSet(grammar).apply(rule); + return FirstSet(&grammar).apply(rule); } set first_set(const ParseItemSet &item_set, const PreparedGrammar &grammar) { diff --git a/src/compiler/build_tables/lex_item.cc b/src/compiler/build_tables/lex_item.cc index a6de7df5..1920b04d 100644 --- a/src/compiler/build_tables/lex_item.cc +++ b/src/compiler/build_tables/lex_item.cc @@ -15,9 +15,7 @@ namespace tree_sitter { Item(lhs, rule) {} bool LexItem::operator==(const LexItem &other) const { - bool lhs_eq = other.lhs == lhs; - bool rules_eq = (*other.rule == *rule); - return lhs_eq && rules_eq; + return (other.lhs == lhs) && other.rule->operator==(*rule); } bool LexItem::is_token_start() const { diff --git a/src/compiler/build_tables/parse_item.cc b/src/compiler/build_tables/parse_item.cc index b851bb05..c41a753f 100644 --- a/src/compiler/build_tables/parse_item.cc +++ b/src/compiler/build_tables/parse_item.cc @@ -17,11 +17,11 @@ namespace tree_sitter { lookahead_sym(lookahead_sym) {} bool ParseItem::operator==(const ParseItem &other) const { - bool lhs_eq = other.lhs == lhs; - bool rules_eq = (*other.rule == *rule); - bool consumed_sym_counts_eq = (other.consumed_symbol_count == consumed_symbol_count); - bool lookaheads_eq = other.lookahead_sym == lookahead_sym; - return lhs_eq && rules_eq && consumed_sym_counts_eq && lookaheads_eq; + return + (other.lhs == lhs) && + (other.consumed_symbol_count == consumed_symbol_count) && + (other.lookahead_sym == lookahead_sym) && + (other.rule->operator==(*rule)); } int ParseItem::precedence() const { diff --git a/src/compiler/build_tables/rule_can_be_blank.cc b/src/compiler/build_tables/rule_can_be_blank.cc index 505f0bdc..2c184e75 100644 --- a/src/compiler/build_tables/rule_can_be_blank.cc +++ b/src/compiler/build_tables/rule_can_be_blank.cc @@ -37,18 +37,18 @@ namespace tree_sitter { }; class CanBeBlankRecursive : public CanBeBlank { - const PreparedGrammar grammar; + const PreparedGrammar *grammar; set visited_symbols; using CanBeBlank::visit; public: using CanBeBlank::apply_to; - explicit CanBeBlankRecursive(const PreparedGrammar &grammar) : grammar(grammar) {} + explicit CanBeBlankRecursive(const PreparedGrammar *grammar) : grammar(grammar) {} bool apply_to(const rules::ISymbol *rule) { if (visited_symbols.find(*rule) == visited_symbols.end()) { visited_symbols.insert(*rule); - return !rule->is_token() && apply(grammar.rule(*rule)); + return !rule->is_token() && apply(grammar->rule(*rule)); } else { return false; } @@ -60,7 +60,7 @@ namespace tree_sitter { } bool rule_can_be_blank(const rules::rule_ptr &rule, const PreparedGrammar &grammar) { - return CanBeBlankRecursive(grammar).apply(rule); + return CanBeBlankRecursive(&grammar).apply(rule); } } } diff --git a/src/compiler/prepared_grammar.cc b/src/compiler/prepared_grammar.cc index fd158536..1e0b04dd 100644 --- a/src/compiler/prepared_grammar.cc +++ b/src/compiler/prepared_grammar.cc @@ -20,13 +20,13 @@ namespace tree_sitter { Grammar(grammar), aux_rules({}) {} - const rule_ptr PreparedGrammar::rule(const ISymbol &symbol) const { + const rule_ptr & PreparedGrammar::rule(const ISymbol &symbol) const { return symbol.is_auxiliary() ? aux_rules[symbol.index].second : rules[symbol.index].second; } - string PreparedGrammar::rule_name(const ISymbol &symbol) const { + const string & PreparedGrammar::rule_name(const ISymbol &symbol) const { return symbol.is_auxiliary() ? aux_rules[symbol.index].first : rules[symbol.index].first; diff --git a/src/compiler/prepared_grammar.h b/src/compiler/prepared_grammar.h index 55f33755..6da53f24 100644 --- a/src/compiler/prepared_grammar.h +++ b/src/compiler/prepared_grammar.h @@ -15,8 +15,8 @@ namespace tree_sitter { PreparedGrammar(const Grammar &grammar); bool operator==(const PreparedGrammar &other) const; - std::string rule_name(const rules::ISymbol &symbol) const; - const rules::rule_ptr rule(const rules::ISymbol &symbol) const; + const std::string & rule_name(const rules::ISymbol &symbol) const; + const rules::rule_ptr & rule(const rules::ISymbol &symbol) const; const std::vector> aux_rules; }; diff --git a/src/compiler/rules/interned_symbol.cc b/src/compiler/rules/interned_symbol.cc index cc2958e1..6c366241 100644 --- a/src/compiler/rules/interned_symbol.cc +++ b/src/compiler/rules/interned_symbol.cc @@ -23,7 +23,7 @@ namespace tree_sitter { } size_t ISymbol::hash_code() const { - return hash()(index) ^ hash()(options); + return hash()(index) ^ hash()(options); } rule_ptr ISymbol::copy() const {