[mvzink]

Some callers of the tokenizer's peeking_take_while helper simply copied the newly-allocated String into an existing String. Refactor it to call a new peeking_read_while helper, and convert 4 take callers to read. This avoids an allocation and extraneous string copy at each callsite.

This was motivated by finding one of the converted callers (tokenize_word) pretty high up when profiling some real workloads. Converting that caller gets most of the improvement in tokenizing time in the added benchmarks, but converting the rest seemed harmless.

Add two benchmarks (results from a few runs on a Ryzen 5 3600):

• mixed_sql: 22 TPC-H files plus eight short statements. ~9-11% improvement to tokenizing time and ~5% improvement to overall parse time.
• tokenize_identifiers benchmark targeting tokenize_word, testing power-of-two identifier lengths from 1 to 128. Pretty contrived, mainly to check this doesn't regress small identifiers. 1-character identifiers showed ~4% reduction in tokenizing time, up to ~14% reduction for 128-character identifiers.

[github-actions]

No dialect label was applied because the title does not name a SQL dialect.

If this change targets one or more dialects, edit the title to <Dialect>[, <Dialect>]: <description>, add a Dialects: <Dialect>[, <Dialect>] line to the description, or reply with one. Reply Dialects: none if the change is dialect-agnostic.

[codecov-commenter]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 82.38%. Comparing base (23e9e88) to head (dafd04e).

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2729      +/-   ##
==========================================
+ Coverage   82.30%   82.38%   +0.08%     
==========================================
  Files          42       42              
  Lines       35294    35405     +111     
  Branches    35294    35405     +111     
==========================================
+ Hits        29047    29167     +120     
+ Misses       2690     2681       -9     
  Partials     3557     3557              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[mvzink]

Dialects: none

[LucaCappelletti94]

Consider that we are targetting a full refactoring of the tokenizer in the next few months, at this time we are primarely focusing on fuzzing-driven bug squashing so that we will be able to tell which errors come from the refactoring and which are just preexisting stuff.

I expect that a good refactoring, expecially aiming to go towards copy-less-ness, will be able to achieve much better performance.

[LucaCappelletti94]

If you have time, we would really appreciate any help in the fuzzing/debugging department & reviews in particular. Also, of course, once the time comes, very welcome to help in any tokenizer refactoring.

I do agree as it stands it is in an unfortunate shape, mostly it is due to an accomulation of patches.

[mvzink]

@LucaCappelletti94 Okay, good to know! I don't think we need this that urgently, and if we do we can use our fork until the refactor comes through. As far as fuzzing etc. are you thinking of validation of the refactor in particular, or the project in general? I am not on Discord any more but I'll try to figure out how to help.

[LucaCappelletti94]

Ah basically there are I believe now 4 different libfuzzer harnesses in the repo. Just pick one that is relevant to the dialect you work on, and start running it. You'll get bugs in no time at all, and gradually we'll get them fixed. Some of the bugs are clearly stuff relative to the tokenization patches, and that is something that as aforementioned will get fixed with a heavy refactoring.

Another thing that would be very helpful is reviews. Just pick any PR you believe you are likely familiar or interested in, and provide a review. That'll move things along.