Skip to content

Change parsing from exponential to linear - #410

Merged
apoelstra merged 2 commits into
BlockstreamResearch:masterfrom
stringhandler:st-parse-linear
Sep 4, 2026
Merged

Change parsing from exponential to linear#410
apoelstra merged 2 commits into
BlockstreamResearch:masterfrom
stringhandler:st-parse-linear

Conversation

@stringhandler

Copy link
Copy Markdown
Contributor

In an attempt to increase the nesting limit in #401, an LLM found this incredibly subtle bug in statements parsing.

It's difficult to explain why this works, basically it would match a block body, then fail and parse each statement again. I've left the extraneous descriptions in the commits, but happy to remove them.

There are two commits.
Commit 1 adds an ignored test that shows the problem. Run it with cargo test --test parser_scaling -- --ignored --nocapture
Commit 2 adds the fix that converts it from exponential to linear. Run the test again afterwards to see the improvement.

Here are some figures from my computer:

running 1 test

  nesting construct, time per depth, and growth across two levels

  blocks
    depth   8     4.85ms       -
    depth  10    16.86ms   3.48x
    depth  12    62.02ms   3.68x
    depth  14   243.23ms   3.92x
    depth  16   989.35ms   4.07x

....
...

  depth 16 against depth 8 (linear is about 2x, doubling per level is about 256x)
    blocks            204.1x  TOO STEEP
    parentheses         1.3x  ok
    option types        1.6x  ok
    tuple types         1.4x  ok
    array types         1.4x  ok
 

After

running 1 test

  nesting construct, time per depth, and growth across two levels

  blocks
    depth   8     1.24ms       -
    depth  10     1.33ms   1.07x
    depth  12     1.26ms   0.95x
    depth  14     1.32ms   1.05x
    depth  16     1.30ms   0.98x

A combinator parser can silently parse the same sub-tree more than once, wherever
two alternatives can both begin at the current token and the choice between them
is only settled after the whole sub-tree has been consumed. One such spot doubles
the work for every level of nesting. Ordinary programs nest a handful of levels
deep, so it shows up there as a small constant factor and nothing in the test
suite notices, while a program a few hundred bytes long takes minutes to parse.

Add a harness that measures each way the grammar can nest -- blocks,
parentheses, and option, tuple and array types -- by parsing each to increasing
depths and reporting how the cost grows:

    cargo test --test parser_scaling -- --ignored --nocapture

It is ignored by default because it times the parser, and timings are too noisy
to gate CI on. It is meant as an instrument to reach for when touching the
grammar, and wants a new case whenever the grammar grows a new way to nest.

Measuring every construct before asserting anything means a failure names which
rule is at fault rather than only that something is slow. Against the parser as
it stands, four of the five constructs are flat and blocks are not:

    blocks            160.5x  TOO STEEP
    parentheses         1.2x  ok
    option types        1.4x  ok
    tuple types         1.4x  ok
    array types         1.3x  ok

The blocks column grows by a factor converging on 4 across every two levels,
which is the doubling per level. The next commit parses each element of a block
body once, after which blocks measure 1.3x alongside the rest.
@stringhandler

Copy link
Copy Markdown
Contributor Author

Unfortunately, for #401 this PR only increases the max nesting from 16 to 32, so more work is needed.

@apoelstra

apoelstra commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

It's difficult to explain why this works

FWIW I think that the commit message in ec211c2 does a good job.

In ec211c2 I think you should remove the #[ignore] on the new test. It runs now in a few ms and will catch regressions since it does this clever "time things and assert that the scaling factor is not too bad". I don't think it will have any false positives because the MAX_RATIO is set so high.

@apoelstra

Copy link
Copy Markdown
Contributor

ec211c2 looks good other than that I think you should unignore the test.

Honestly the new code is easier to follow than the old (once you look up what .or_not and .then do in Chumsky).

I might suggest adding type annotations to most of the let bindings in this module so that it's clearer what's an option, what's a tuple, etc. But we don't need to do that in this PR.

A block body was parsed as "statements, then an optional final expression",
which are two parsers that both start at the same token. Every element of a
block was therefore parsed twice: once as a statement, which only fails after
the whole element has been consumed and no `;` turns up, and then again as the
final expression. That doubles the work for each level of block nesting, so
parsing is exponential in how deeply a program nests blocks.

Measured on a release build before this change, a program that nests nothing
but blocks takes 315ms at depth 16, 5.5s at depth 20 and 82s at depth 24, and
does not finish within five minutes at depth 28. Any service that compiles
submitted .simf files can be stalled by a few hundred bytes of input.

Parse each element once instead and decide what it was from the `;` that
follows it: only the last element may drop its `;`, and only when it is an
expression rather than an assignment, in which case it is the block's final
expression. A missing `;` anywhere else is reported and the element is kept as
a statement, so the rest of the block is still analyzed.

The same nesting now parses in 2ms at depth 24 and 18ms at depth 512.
@stringhandler

Copy link
Copy Markdown
Contributor Author

Updated. The test runs quicker than some others.

@apoelstra apoelstra left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK e2530af; successfully ran local tests

@apoelstra
apoelstra merged commit c31357c into BlockstreamResearch:master Sep 4, 2026
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants