Perl_study_chunk - static helpers for flattening nested branches - #24611
Perl_study_chunk - static helpers for flattening nested branches#24611richardleach wants to merge 2 commits into
Conversation
ba380d8 to
b1b244a
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.
It does not flatten more complicated examples, such as:
/(?:(?:frog|fan)|fog)|(?:farce|(?:forge|flambe))/
It also does not flatten _some_ nested branches when flattening _all_
nested branches would overflow a TRIE node; it's currently all or
nothing.
Future enhancements might be able to go further.
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. |
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.
It does not flatten more complicated examples, such as:
It also does not flatten some nested branches when flattening all
nested branches would overflow a TRIE node; it's currently all or
nothing.
Future enhancements might be able to go further.
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!