Skip to content

Commit daee7bf

Browse files
Minor grammar refactor for parser performance improvements (#439)
## Product change and motivation Reordering the choices to boost performance by failing faster and having more frequent alternatives earlier. Also removes the `!reserved` check for identifiers and expects this to be handled in an application post-check.
1 parent 8e2524a commit daee7bf

11 files changed

Lines changed: 326 additions & 75 deletions

File tree

MODULE.bazel.lock

Lines changed: 20 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

rust/Cargo.toml

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,11 +24,26 @@ features = {}
2424
version = "1.47.1"
2525
default-features = false
2626

27+
[dev-dependencies.tracing]
28+
features = ["attributes", "default", "log", "std", "tracing-attributes"]
29+
version = "0.1.41"
30+
default-features = false
31+
32+
[dev-dependencies.criterion]
33+
features = ["cargo_bench_support", "default", "plotters", "rayon"]
34+
version = "0.5.1"
35+
default-features = false
36+
2737
[dev-dependencies.cucumber]
2838
features = ["default", "macros"]
2939
version = "0.19.1"
3040
default-features = false
3141

42+
[dev-dependencies.pprof]
43+
features = ["cpp", "criterion", "default", "flamegraph", "inferno"]
44+
version = "0.13.0"
45+
default-features = false
46+
3247
[dev-dependencies.syn]
3348
features = ["clone-impls", "default", "derive", "extra-traits", "fold", "full", "parsing", "printing", "proc-macro", "visit", "visit-mut"]
3449
version = "2.0.106"
@@ -76,6 +91,10 @@ features = {}
7691
version = "0.10.5"
7792
default-features = false
7893

94+
[[bench]]
95+
name = "bench_parser"
96+
harness = false
97+
7998
[[test]]
8099
path = "tests/behaviour/test_behaviour.rs"
81100
name = "test_behaviour"

rust/benches/BUILD

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
# This Source Code Form is subject to the terms of the Mozilla Public
2+
# License, v. 2.0. If a copy of the MPL was not distributed with this
3+
# file, You can obtain one at https://mozilla.org/MPL/2.0/.
4+
5+
load("@typedb_dependencies//tool/checkstyle:rules.bzl", "checkstyle_test")
6+
load("@rules_rust//rust:defs.bzl", "rust_test", "rustfmt_test")
7+
package(default_visibility = ["//visibility:public",])
8+
9+
# To run this via Bazel, Criterion must be provided the --bench argument:
10+
# bazel run --compilation_mode=opt //rust/parser:bench_parser -- --bench
11+
rust_test(
12+
name = "bench_parser",
13+
srcs = ["bench_parser.rs"],
14+
deps = [
15+
"//rust:typeql",
16+
17+
"@crates//:rand",
18+
"@crates//:criterion",
19+
"@crates//:pprof",
20+
],
21+
tags = ["manual"], # in order for bazel test //... to not fail
22+
use_libtest_harness = False,
23+
)
24+
25+
checkstyle_test(
26+
name = "checkstyle",
27+
include = glob([
28+
"*",
29+
]),
30+
license_type = "mpl-header",
31+
)

rust/benches/bench_parser.rs

Lines changed: 116 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,116 @@
1+
/*
2+
* This Source Code Form is subject to the terms of the Mozilla Public
3+
* License, v. 2.0. If a copy of the MPL was not distributed with this
4+
* file, You can obtain one at https://mozilla.org/MPL/2.0/.
5+
*/
6+
7+
use std::{fs::File, os::raw::c_int, path::Path};
8+
9+
use criterion::{criterion_group, criterion_main, profiler::Profiler, Criterion};
10+
use pprof::ProfilerGuard;
11+
12+
const N_LINES: usize = 1000;
13+
fn get_query_isa() -> String {
14+
let mut query: String = "match\n".to_owned();
15+
(0..N_LINES).for_each(|_| query.push_str("$v_abc123 isa t_abc123 == 123456;\n"));
16+
query
17+
}
18+
19+
fn get_query_var_sub() -> String {
20+
let mut query: String = "match\n".to_owned();
21+
(0..N_LINES).for_each(|_| query.push_str("$v_abc123 sub t_abc123;\n"));
22+
query
23+
}
24+
25+
fn get_query_label_sub() -> String {
26+
let mut query: String = "match\n".to_owned();
27+
(0..N_LINES).for_each(|_| query.push_str("t_pqr456 sub t_abc123;\n"));
28+
query
29+
}
30+
31+
fn get_query_expression() -> String {
32+
let mut query: String = "match\n".to_owned();
33+
(0..N_LINES).for_each(|_| query.push_str("let $volume = (4/3) * 3.14 * pow($r, 3) ;\n"));
34+
query
35+
}
36+
37+
fn get_query_disjunctions() -> String {
38+
let stmt = "$v_abc123 isa t_abc123 == 123456;\n";
39+
let line = format!("{{ {stmt} }} or\n");
40+
41+
let mut query: String = "match\n".to_owned();
42+
(0..(N_LINES - 1)).for_each(|_| query.push_str(line.as_str()));
43+
query.push_str(format!("{{ {stmt} }};").as_str());
44+
query
45+
}
46+
47+
fn get_query_many_inserts() -> String {
48+
let mut query: String = "".to_owned();
49+
(0..N_LINES).for_each(|_| query.push_str("insert $_ isa person;\n"));
50+
query
51+
}
52+
53+
fn get_define_label_sub() -> String {
54+
let mut query: String = "define\n".to_owned();
55+
(0..N_LINES).for_each(|_| query.push_str("t_pqr456 sub t_abc123;\n"));
56+
query
57+
}
58+
59+
fn criterion_benchmark(c: &mut Criterion) {
60+
println!("In criterion benchmark");
61+
let query_isa = get_query_isa();
62+
let query_label_sub = get_query_isa();
63+
let query_var_sub = get_query_var_sub();
64+
let query_expression = get_query_expression();
65+
let query_disjunctions = get_query_disjunctions();
66+
let query_many_inserts = get_query_many_inserts();
67+
let define_label_sub = get_define_label_sub();
68+
c.bench_function("parse-isa", |b| b.iter(|| typeql::parse_query(query_isa.as_str()).unwrap()));
69+
c.bench_function("parse-var-sub", |b| b.iter(|| typeql::parse_query(query_var_sub.as_str()).unwrap()));
70+
c.bench_function("parse-label-sub", |b| b.iter(|| typeql::parse_query(query_label_sub.as_str()).unwrap()));
71+
c.bench_function("parse-expression", |b| b.iter(|| typeql::parse_query(query_expression.as_str()).unwrap()));
72+
c.bench_function("parse-disjunctions", |b| b.iter(|| typeql::parse_query(query_disjunctions.as_str()).unwrap()));
73+
c.bench_function("parse-many-inserts", |b| b.iter(|| typeql::parse_query(query_many_inserts.as_str()).unwrap()));
74+
c.bench_function("parse-define-sub", |b| b.iter(|| typeql::parse_query(define_label_sub.as_str()).unwrap()));
75+
}
76+
// --- Code to generate flamegraphs copied from https://www.jibbow.com/posts/criterion-flamegraphs/ ---
77+
// This causes a SIGBUS on (mac) arm64 if the frequency is set too high.
78+
79+
pub struct FlamegraphProfiler<'a> {
80+
frequency: c_int,
81+
active_profiler: Option<ProfilerGuard<'a>>,
82+
}
83+
84+
impl<'a> FlamegraphProfiler<'a> {
85+
#[allow(dead_code)]
86+
pub fn new(frequency: c_int) -> Self {
87+
FlamegraphProfiler { frequency, active_profiler: None }
88+
}
89+
}
90+
91+
impl<'a> Profiler for FlamegraphProfiler<'a> {
92+
fn start_profiling(&mut self, _benchmark_id: &str, _benchmark_dir: &Path) {
93+
self.active_profiler = Some(ProfilerGuard::new(self.frequency).unwrap());
94+
}
95+
96+
fn stop_profiling(&mut self, _benchmark_id: &str, benchmark_dir: &Path) {
97+
std::fs::create_dir_all(benchmark_dir).unwrap();
98+
let flamegraph_path = benchmark_dir.join("flamegraph.svg");
99+
let flamegraph_file = File::create(flamegraph_path).expect("File system error while creating flamegraph.svg");
100+
if let Some(profiler) = self.active_profiler.take() {
101+
profiler.report().build().unwrap().flamegraph(flamegraph_file).expect("Error writing flamegraph");
102+
}
103+
}
104+
}
105+
106+
fn profiled() -> Criterion {
107+
Criterion::default().with_profiler(FlamegraphProfiler::new(100))
108+
}
109+
110+
criterion_group!(
111+
name = benches;
112+
config = profiled();
113+
targets = criterion_benchmark
114+
);
115+
116+
criterion_main!(benches);

rust/parser/mod.rs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -276,3 +276,10 @@ fn visit_reduce_assignment_var(node: Node<'_>) -> Variable {
276276
_ => unreachable!("{}", TypeQLError::IllegalGrammar { input: child.as_str().to_owned() }),
277277
}
278278
}
279+
280+
#[cfg(test)]
281+
pub mod tests {
282+
pub fn is_keyword(identifier: &str) -> bool {
283+
super::parse(super::Rule::reserved, identifier).is_ok()
284+
}
285+
}

rust/parser/statement/mod.rs

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4,13 +4,14 @@
44
* file, You can obtain one at https://mozilla.org/MPL/2.0/.
55
*/
66

7-
use self::single::visit_statement_single;
8-
use super::{
9-
expression::visit_expression_value, statement::type_::visit_statement_type, IntoChildNodes, Node, Rule, RuleMatcher,
7+
use self::{
8+
single::{visit_statement_assignment, visit_statement_comparison, visit_statement_in, visit_statement_is},
9+
thing::{visit_statement_thing_basic, visit_statement_thing_relation_anonymous},
10+
type_::visit_statement_type,
1011
};
12+
use super::{expression::visit_expression_value, IntoChildNodes, Node, Rule, RuleMatcher};
1113
use crate::{
1214
common::{error::TypeQLError, token::Comparator, Spanned},
13-
parser::statement::thing::visit_statement_thing,
1415
statement::{comparison::Comparison, Statement},
1516
};
1617

@@ -22,9 +23,13 @@ pub(super) fn visit_statement(node: Node<'_>) -> Statement {
2223
debug_assert_eq!(node.as_rule(), Rule::statement);
2324
let child = node.into_child();
2425
match child.as_rule() {
25-
Rule::statement_single => visit_statement_single(child),
26+
Rule::statement_thing_basic => visit_statement_thing_basic(child),
27+
Rule::statement_thing_relation_anonymous => visit_statement_thing_relation_anonymous(child),
2628
Rule::statement_type => visit_statement_type(child),
27-
Rule::statement_thing => visit_statement_thing(child),
29+
Rule::statement_is => Statement::Is(visit_statement_is(child)),
30+
Rule::statement_in => Statement::InIterable(visit_statement_in(child)),
31+
Rule::statement_comparison => Statement::Comparison(visit_statement_comparison(child)),
32+
Rule::statement_assignment => Statement::Assignment(visit_statement_assignment(child)),
2833
_ => unreachable!("{}", TypeQLError::IllegalGrammar { input: child.as_str().to_owned() }),
2934
}
3035
}

rust/parser/statement/single.rs

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -20,18 +20,6 @@ use crate::{
2020
},
2121
};
2222

23-
pub(super) fn visit_statement_single(node: Node<'_>) -> Statement {
24-
debug_assert_eq!(node.as_rule(), Rule::statement_single);
25-
let child = node.into_child();
26-
match child.as_rule() {
27-
Rule::statement_is => Statement::Is(visit_statement_is(child)),
28-
Rule::statement_in => Statement::InIterable(visit_statement_in(child)),
29-
Rule::statement_comparison => Statement::Comparison(visit_statement_comparison(child)),
30-
Rule::statement_assignment => Statement::Assignment(visit_statement_assignment(child)),
31-
_ => unreachable!("{}", TypeQLError::IllegalGrammar { input: child.as_str().to_owned() }),
32-
}
33-
}
34-
3523
pub fn visit_statement_assignment(node: Node<'_>) -> Assignment {
3624
debug_assert_eq!(node.as_rule(), Rule::statement_assignment);
3725
let span = node.span();

rust/parser/statement/thing.rs

Lines changed: 16 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -25,39 +25,28 @@ use crate::{
2525
TypeRef,
2626
};
2727

28-
pub(in crate::parser) fn visit_statement_thing(node: Node<'_>) -> Statement {
29-
debug_assert_eq!(node.as_rule(), Rule::statement_thing);
28+
pub(in crate::parser) fn visit_statement_thing_basic(node: Node<'_>) -> Statement {
29+
debug_assert_eq!(node.as_rule(), Rule::statement_thing_basic);
3030
let span = node.span();
3131
let mut children = node.into_children();
32-
let child = children.consume_any();
33-
match child.as_rule() {
34-
Rule::var => {
35-
let var = visit_var(child);
36-
let constraints = visit_thing_constraint_list(children.consume_expected(Rule::thing_constraint_list));
37-
Statement::Thing(Thing::new(span, Head::Variable(var), constraints))
38-
}
39-
Rule::thing_relation_anonymous => {
40-
let (type_ref_opt, relation) = visit_thing_relation_anonymous(child);
41-
let constraints = if let Some(constraint_list) = children.try_consume_expected(Rule::thing_constraint_list)
42-
{
43-
visit_thing_constraint_list(constraint_list)
44-
} else {
45-
debug_assert_eq!(children.try_consume_any(), None);
46-
vec![]
47-
};
48-
Statement::Thing(Thing::new(span, Head::Relation(type_ref_opt, relation), constraints))
49-
}
50-
_ => unreachable!("{}", TypeQLError::IllegalGrammar { input: child.as_str().to_owned() }),
51-
}
32+
let var = visit_var(children.consume_expected(Rule::var));
33+
let constraints = visit_thing_constraint_list(children.consume_expected(Rule::thing_constraint_list));
34+
Statement::Thing(Thing::new(span, Head::Variable(var), constraints))
5235
}
5336

54-
pub(super) fn visit_thing_relation_anonymous(node: Node<'_>) -> (Option<TypeRef>, Relation) {
55-
debug_assert_eq!(node.as_rule(), Rule::thing_relation_anonymous);
56-
let _span = node.span();
37+
pub(super) fn visit_statement_thing_relation_anonymous(node: Node<'_>) -> Statement {
38+
debug_assert_eq!(node.as_rule(), Rule::statement_thing_relation_anonymous);
39+
let span = node.span();
5740
let mut children = node.into_children();
58-
let type_ = children.try_consume_expected(Rule::type_ref).map(visit_type_ref);
41+
let type_ref_opt = children.try_consume_expected(Rule::type_ref).map(visit_type_ref);
5942
let relation = visit_relation(children.consume_expected(Rule::relation));
60-
(type_, relation)
43+
let constraints = if let Some(constraint_list) = children.try_consume_expected(Rule::thing_constraint_list) {
44+
visit_thing_constraint_list(constraint_list)
45+
} else {
46+
debug_assert_eq!(children.try_consume_any(), None);
47+
vec![]
48+
};
49+
Statement::Thing(Thing::new(span, Head::Relation(type_ref_opt, relation), constraints))
6150
}
6251

6352
fn visit_thing_constraint_list(node: Node<'_>) -> Vec<Constraint> {

rust/parser/test/match_queries.rs

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,26 @@ select $char, $prod;"#;
6666
assert_valid_eq_repr!(expected, parsed, query);
6767
}
6868

69+
#[test]
70+
fn test_labelled_relation_without_role() {
71+
let query = r#"match
72+
$brando isa name "Marl B";
73+
casting ($brando, $char, $prod);
74+
select $char, $prod;"#;
75+
let parsed = parse_query(query).unwrap();
76+
assert_valid_eq_repr!(expected, parsed, query);
77+
}
78+
79+
#[test]
80+
fn test_comparison_which_looks_like_relation() {
81+
let query = r#"match
82+
$brando isa name "Marl B";
83+
somefunc($brando, $char, $prod) > 5;
84+
select $char, $prod;"#;
85+
let parsed = parse_query(query).unwrap();
86+
assert_valid_eq_repr!(expected, parsed, query);
87+
}
88+
6989
#[test]
7090
fn test_role_type_scoped_globally() {
7191
let query = r#"match

0 commit comments

Comments
 (0)