Uh oh!
There was an error while loading. Please reload this page.
Add single char lazy loop support to simplified Regex code gen - #61698
Conversation
ghost
commented
Nov 16, 2021
Tagging subscribers to this area: @eerhardt, @dotnet/area-system-text-regularexpressions Issue Details#60385 added support for greedy loops to our "simplified" code gen for the Regex compiler and source generator. This does the same for lazy loops. In our stable of known real-world regex patterns, this increases the number supported by the simplified code gen by ~5% to ~70%. (Note we can subsequently add vectorization into this to, for example, special-case .* and use IndexOf{Any} to search for what comes next.) cc: @joperezr, @danmoseley Code generated for `<.*?>` beforeprotectedoverridevoidGo(){stringruntext=base.runtext!;intruntextbeg=base.runtextbeg;intruntextend=base.runtextend;intruntextpos=base.runtextpos;int[]runtrack=base.runtrack!;intruntrackpos=base.runtrackpos;int[]runstack=base.runstack!;intrunstackpos=base.runstackpos;inttmp1,tmp2,ch;// 000000 *Lazybranch addr = 13L0:runtrack[--runtrackpos]=runtextpos;runtrack[--runtrackpos]=0;// 000002 *SetmarkL1:runstack[--runstackpos]=runtextpos;runtrack[--runtrackpos]=1;// 000003 One '<'L2:if(runtextpos>=runtextend||runtext[runtextpos++]!=60){gotoBacktrack;}// 000005 *Notonelazy '\\n', rep = infL3:tmp1=runtextend-runtextpos;// countif(tmp1<=0){gotoL4;}runtrack[--runtrackpos]=tmp1-1;runtrack[--runtrackpos]=runtextpos;runtrack[--runtrackpos]=2;// 000008 One '>'L4:if(runtextpos>=runtextend||runtext[runtextpos++]!=62){gotoBacktrack;}// 000010 *Capturemark index = 0L5:tmp1=runstack[runstackpos++];base.Capture(0,tmp1,runtextpos);runtrack[--runtrackpos]=tmp1;runtrack[--runtrackpos]=3;// 000013 StopL6:base.runtextpos=runtextpos;return;Backtrack:intlimit=base.runtrackcount*4;if(runstackpos<limit){base.runstackpos=runstackpos;base.DoubleStack();// might change runstackpos and runstackrunstackpos=base.runstackpos;runstack=base.runstack!;}if(runtrackpos<limit){base.runtrackpos=runtrackpos;base.DoubleTrack();// might change runtrackpos and runtrackruntrackpos=base.runtrackpos;runtrack=base.runtrack!;}switch(runtrack[runtrackpos++]){case0:{// 000000 *Lazybranch addr = 13runtextpos=runtrack[runtrackpos++];gotoL6;}case1:{// 000002 *Setmarkrunstackpos++;gotoBacktrack;}case2:{// 000005 *Notonelazy '\\n', rep = infruntextpos=runtrack[runtrackpos++];tmp1=runtrack[runtrackpos++];// iif(runtext[runtextpos++]=='\n'){gotoBacktrack;}if(tmp1>0){runtrack[--runtrackpos]=tmp1-1;runtrack[--runtrackpos]=runtextpos;runtrack[--runtrackpos]=2;}gotoL4;}case3:{// 000010 *Capturemark index = 0runstack[--runstackpos]=runtrack[runtrackpos++];base.Uncapture();gotoBacktrack;}default:{global::System.Diagnostics.Debug.Fail($"Unexpected backtracking state {runtrack[runtrackpos-1]}");break;}}}Code generated for `<.*?>` afterprotectedoverridevoidGo(){stringruntext=base.runtext!;intruntextpos=base.runtextpos;intruntextend=base.runtextend;intoriginalruntextpos=runtextpos;global::System.ReadOnlySpan<byte>byteSpan;charch;global::System.ReadOnlySpan<char>textSpan=global::System.MemoryExtensions.AsSpan(runtext,runtextpos,runtextend-runtextpos);// Concatenate//{// One '<'{if((uint)textSpan.Length<1||textSpan[0]!='<'){gotoNoMatch;}}// Notonelazy '\\n'*//{runtextpos++;textSpan=textSpan.Slice(1);intnextPos0=runtextpos;gotoendLoop0;Backtrack1:runtextpos=nextPos0;textSpan=global::System.MemoryExtensions.AsSpan(runtext,runtextpos,runtextend-runtextpos);if((uint)textSpan.Length<1||textSpan[0]=='\n'){gotoNoMatch;}runtextpos++;textSpan=textSpan.Slice(1);nextPos0=runtextpos;endLoop0://}// One '>'{if((uint)textSpan.Length<1||textSpan[0]!='>'){gotoBacktrack1;}}//}// Matchruntextpos++;base.runtextpos=runtextpos;base.Capture(0,originalruntextpos,runtextpos);return;// No matchNoMatch:return;}
|
77ae85a to
947a861CompareUh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
joperezr
left a comment
There was a problem hiding this comment.
Left a couple of questions, but this looks good.
Random question: is there a reason why we wouldn't want this optimization for multi lazy loops too? E.g. There seems to be a very BIG difference between codegen from onelazy loops and multilazy loops: Code generated for `(?:a)*?b`protectedoverridevoidGo(){stringruntext=base.runtext!;intruntextpos=base.runtextpos;intruntextend=base.runtextend;intoriginalruntextpos=runtextpos;global::System.ReadOnlySpan<byte>byteSpan;charch;global::System.ReadOnlySpan<char>textSpan=global::System.MemoryExtensions.AsSpan(runtext,runtextpos,runtextend-runtextpos);// Concatenate//{// Onelazy 'a'*//{intnextPos0=runtextpos;gotoendLoop0;Backtrack1:runtextpos=nextPos0;textSpan=global::System.MemoryExtensions.AsSpan(runtext,runtextpos,runtextend-runtextpos);if((uint)textSpan.Length<1||textSpan[0]!='a'){gotoNoMatch;}runtextpos++;textSpan=textSpan.Slice(1);nextPos0=runtextpos;endLoop0://}// One 'b'{if((uint)textSpan.Length<1||textSpan[0]!='b'){gotoBacktrack1;}}//}// Matchruntextpos++;base.runtextpos=runtextpos;base.Capture(0,originalruntextpos,runtextpos);return;// No matchNoMatch:return;}Code generated for `(?:ab)*?b`protectedoverridevoidGo(){stringruntext=base.runtext!;intruntextbeg=base.runtextbeg;intruntextend=base.runtextend;intruntextpos=base.runtextpos;int[]runtrack=base.runtrack!;intruntrackpos=base.runtrackpos;int[]runstack=base.runstack!;intrunstackpos=base.runstackpos;inttmp1,tmp2,ch;// 000000 *Lazybranch addr = 15L0:runtrack[--runtrackpos]=runtextpos;runtrack[--runtrackpos]=0;// 000002 *SetmarkL1:runstack[--runstackpos]=runtextpos;runtrack[--runtrackpos]=1;// 000003 NullmarkL2:runstack[--runstackpos]=-1;runtrack[--runtrackpos]=1;// 000004 *Goto addr = 8L3:gotoL5;// 000006 Multi "ab"L4:if(runtextend-runtextpos<2||runtext[runtextpos]!='a'||runtext[runtextpos+1]!='b'){gotoBacktrack;}runtextpos+=2;// 000008 *Lazybranchmark addr = 6L5:tmp1=runstack[runstackpos++];// markruntrack[--runtrackpos]=tmp1!=-1?tmp1:runtextpos;if(runtextpos!=tmp1){runtrack[--runtrackpos]=runtextpos;runtrack[--runtrackpos]=2;gotoL6;}runstack[--runstackpos]=tmp1;runtrack[--runtrackpos]=3;// 000010 One 'b'L6:if(runtextpos>=runtextend||runtext[runtextpos++]!=98){gotoBacktrack;}// 000012 *Capturemark index = 0L7:tmp1=runstack[runstackpos++];base.Capture(0,tmp1,runtextpos);runtrack[--runtrackpos]=tmp1;runtrack[--runtrackpos]=4;// 000015 StopL8:base.runtextpos=runtextpos;return;Backtrack:intlimit=base.runtrackcount*4;if(runstackpos<limit){base.runstackpos=runstackpos;base.DoubleStack();// might change runstackpos and runstackrunstackpos=base.runstackpos;runstack=base.runstack!;}if(runtrackpos<limit){base.runtrackpos=runtrackpos;base.DoubleTrack();// might change runtrackpos and runtrackruntrackpos=base.runtrackpos;runtrack=base.runtrack!;}switch(runtrack[runtrackpos++]){case0:{// 000000 *Lazybranch addr = 15runtextpos=runtrack[runtrackpos++];gotoL8;}case1:{// 000002 *Setmarkrunstackpos++;gotoBacktrack;}case2:{// 000008 *Lazybranchmark addr = 6runtextpos=runtrack[runtrackpos++];runstack[--runstackpos]=runtextpos;runtrack[--runtrackpos]=3;if(runtrackpos<=20||runstackpos<=15){runtrack[--runtrackpos]=5;gotoBacktrack;}gotoL4;}case3:{// 000008 *Lazybranchmark addr = 6runstack[runstackpos]=runtrack[runtrackpos++];gotoBacktrack;}case4:{// 000012 *Capturemark index = 0runstack[--runstackpos]=runtrack[runtrackpos++];base.Uncapture();gotoBacktrack;}case5:{gotoL4;}default:{global::System.Diagnostics.Debug.Fail($"Unexpected backtracking state {runtrack[runtrackpos-1]}");break;}}}Of course I'm not suggesting for that support to also be added here if this is something we do want to add, but just asking in case we want to log an issue for adding that later. |
stephentoub
commented
Nov 18, 2021
"This optimization" being the simplified code gen path? Eventually it'd be nice if we could switch to this approach for everything. I've been building it up piecemeal, focusing on which constructs would cast the widest net. In .NET 5, we covered ~40% of real-world patterns (based on our data set). From previous PRs in .NET 7, we've gotten that up to ~65%. This PR gets us to ~70%. The construct you're talking about shows up as a RegexNode.Lazy, which wraps an arbitrary node, rather than a RegexNode.{One/Notone/Set}lazy, which is a specialized, very popular (in particular sets) single character lazy loop. This PR adds support for the latter; we'd need to do something different for the former. The code to handle a RegexNode.Loop (greedy) would be doable but I expect comparatively non-trivial and involve a true stack, as we'd need to track all the places each iteration completed in order to backtrack there... but for RegexNode.Lazy, it's probably a lot simpler and much closer to this PR's code, since we're incrementally matching another iteration and thus don't need to track an arbitrary number of locations. If you're interested in dipping your toes in, might be a good one to attempt (according to our "real-world" data set, it would net us less than 1% additional patterns, but that doesn't say anything about how "valuable" those patterns are). |
947a861 to
b051c2cComparestephentoub
commented
Nov 18, 2021
Actually, looking at it further, I think it's just a few lines needing to be tweaked on top of what this PR provided... trying... |
#60385 added support for greedy loops to our "simplified" code gen for the Regex compiler and source generator. This does the same for lazy loops. In our stable of known real-world regex patterns, this increases the number supported by the simplified code gen by ~5% to ~70%.
(Note we can subsequently add vectorization into this to, for example, special-case .* and use IndexOf{Any} to search for what comes next.)
cc: @joperezr, @danmoseley
Code generated for `<.*?>` before
Code generated for `<.*?>` after