[EPIC] Improve sqlparser performance

[alamb]

What problem are you trying to solve?

Normally, in a SQL processing system, parsing SQL is not a major bottleneck compared to actually processing data. That being said, given how many SQL strings are parsed by this crate, I think there is significant benefit to improving the performance of the SQL parser in this crate.

That being said, I also think it is important to minimize the impact on downstream crates as much as possible.

Recently, we started introducing locations into the parser (thanks again @Nyrox!), which we found slows things down a bit (see #1435 (comment)).

Thankfully, I think there is significant room for improvement. As as part of the adding location information, I spent some time profiling and I think there are some obvious ways to improve the performance without impacting downstream crates.

Here is the flamegraph for anyone who is interested (you can download it locally to get zoom / etc):

fixed-flamegraph

What would you like to see?

The idea would be

1. Run the benchmarks (instructions in Document micro benchmarks #1555)
2. Maybe add additional benchmarks so they are more representative
3. Improve the benchmarks

Ideas to improve performance:

• The most obvious one is to next_token / peek to not clone each Token (which involves copying strings)L: Improve performance by not copying Tokens as much #1558
• make Parser generic around dialect #1381

[davisp]

@alamb How are you generating your flamegraphs?

Locally, cargo flamegraph --bench sqlparser_bench does not appear to be doing the trick as the flamegraph appears to be at most measuring a single iteration of the benchmark.

[alamb]

Locally, cargo flamegraph --bench sqlparser_bench does not appear to be doing the trick as the flamegraph appears to be at most measuring a single iteration of the benchmark.

This is basically what I normally do. Sometimes it is tricky to interpret the cargo bench flamegraph as it has both the warmup and the run

Can you post the flamegraph you have?

[davisp]

So this is kinda weird. It appears the --bench argument that should be passed to the benchmark isn't.

Running cargo flamegraph --root --bench sqlparser_bench:

However, it works fine if I run flamegraph against the binary directly:

flamegraph --root -- target/release/deps/sqlparser_bench-9f7a6e6e193d8f5c --bench

And the easier to remember version works as well:

cargo flamegraph --root --bench sqlparser_bench -- --bench:

So no idea why that --bench is apparently getting dropped but I've managed to get things going at least.

[alamb]

So no idea why that --bench is apparently getting dropped but I've managed to get things going at least.

🎉 glad to hear you figured out a way past the current issue. I have also found argument handling with cargo bench a little strange so I don't have a great explanation 🤷