Regex compilation - static helpers for flattening nested branches - #24611
Conversation
b1b244a to
e038d5d
Compare
|
Nice! I really like this. Thanks! I only gave it a quick look now, and it needs @khwilliamson to review as well, but it makes sense to me at a high level for sure. I am curious why the flattening isn't recursive. Is it performance reasons? Or keeping track of the number of items involved? Off the top of my head id expect that study_chunk (Which is effectively depth first) would find the leafmost nested caste, linearize them, and then repeat the process as it returned back up the tree, flattening them all. This really reminds me of something that we do (and which @khwilliamson rewrote IIRC), which was gathering adjacent single char EXACTish nodes into single multi-char nodes. What you are doing there is conceptually quite similar. I actually would say we should just flatten always, even regardless of what the trie would do. |
|
I had a look at this: which currently triggers the common prefix extraction, and actually turns into this: which demonstrates the depth firstness of the procedure. the common prefix extraction in the TRIE code turns I think this depth first processing nature is going to get in the way of arbitrary flattening. It may be better to do this during the parse phase. If you look at the The output on the last line there, (and similar above) mentioning 'tying lastbr ... to ender' is the debug output of taking all the branches in an alternation and trying their tail to the right node (which cant be determined when the node is parsed) . Consider BTW, the general pattern for this is as follows, S_reg() calls S_regbranch() to parse each branch (every regex is assumed to be an alternation with one branch to start off with), when there is more than one such branch, the next point of each BRANCH node is tied to the next BRANCH in the sequence, and S_reg() keeps track of the first node in the sequence. When S_reg() encounters a close paren (or pattern end) and it has a set of branches waiting, it ties their end branch to the subsequent node. The point being the code is actually probably structured petty well to move the flattening logic to parse time which in turn would make it more independent from the trie logic and make the flattening arbitrarily deep. NB. It is interesting to note that if we were doing full DFA construction all of this would just come out in the wash. We would construct the same DFA regardless. |
Thanks, that does sound like a better place to attempt this flattening. I'll have a go. |
An attempt at doing this in |
e038d5d to
fee4582
Compare
|
Still could do with a bit of refinement, but I've pushed for CI and any comments at this stage. |
richardleach
changed the title
a3127c4 to
4e79c8b
Compare
4e79c8b to
7a4ff5e
Compare
107ce17 to
eb49208
Compare
This commit attempts to flatten BRANCHes within a BRANCH, mostly for the
benefit of conversion of EXACT alternations into fewer TRIE nodes.
For example, prior to this commit, the following pattern:
/mat|mat2|(?:mat3|mat4)|mat5|(?:mat6|mat7)/
would compile to:
1: TRIEC-EXACT[m] (35)
<mat> (35)
<mat2> (35)
<mat> (13)
13: TRIE-EXACT[34] (35)
<3>
<4>
<mat5> (35)
<mat> (28)
28: TRIE-EXACT[67] (35)
<6>
<7>
35: END (0)
now it compiles to:
1: EXACT <mat> (3)
3: TRIE-EXACT[2-7] (35)
<>
<2>
<3>
<4>
<5>
<6>
<7>
35: END (0)
The commit only flattens branches where the branch tails directly match.
Exactly when and to what extent inner branches get flattened may change
as this behaviour is further refined.
eb49208 to
a858a77
Compare
|
Thanks a lot @richardleach . |
2 participants
This commit attempts to flatten BRANCHes within a BRANCH, mostly for the
benefit of conversion of EXACT alternations into fewer TRIE nodes.
For example, prior to this commit, the following pattern:
would compile to:
now it compiles to:
The commit only flattens branches where the branch tails directly match.
Update: Following the suggestion to implement in
S_reg, more complicated examples get flattened than was the case with the original attempt.Notes:
1. While a person might not write that sort of branching pattern,
it could arise from combining RE pieces, otherwise programatically
generating a pattern, or (potentially) from earlier parsing of
non-explicit branches into BRANCH regnodes.
2. I'm no regex engine guru. There may be a better way / place to
do this, or the implementation might be as suboptimal as my
understanding of regcomp. In particular, I was unsure if or how
to apply to BRANCHJ regnodes.
3. Also, any suggestions for improving the tests would be
welcomed!