Uh oh!
There was an error while loading. Please reload this page.
Remove some unnecessary slicing from generated Regex code - #61701
Conversation
When we're outputting code to match a "multi" (a sequence of multiple characters), we're currently issuing a Slice for the known tracked offset even if that offset is 0. We can skip that nop.
ghost
commented
Nov 16, 2021
Tagging subscribers to this area: @eerhardt, @dotnet/area-system-text-regularexpressions Issue DetailsWhen we're outputting code to match a "multi" (a sequence of multiple characters), we're currently issuing a Slice for the known tracked offset even if that offset is 0. We can skip that nop. cc: @joperezr, @danmoseley Matching "defg" as part of a pattern before// Multi "defg"{byteSpan=global::System.Runtime.InteropServices.MemoryMarshal.AsBytes(textSpan);if((uint)textSpan.Length<4||global::System.Buffers.Binary.BinaryPrimitives.ReadUInt64LittleEndian(byteSpan.Slice(0))!=0x67006600650064ul){gotoNoMatch;}}Matching "defg" as part of a pattern after// Multi "defg"{byteSpan=global::System.Runtime.InteropServices.MemoryMarshal.AsBytes(textSpan);if((uint)textSpan.Length<4||global::System.Buffers.Binary.BinaryPrimitives.ReadUInt64LittleEndian(byteSpan)!=0x67006600650064ul){gotoNoMatch;}}
|
Uh oh!
There was an error while loading. Please reload this page.
joperezr
left a comment
There was a problem hiding this comment.
Other than the one comment this looks good. Is it worth adding a unit test that ensures the Slice is not there in this case? (if the tiny optimization doesn't warrant a test that's ok too)
stephentoub
commented
Nov 17, 2021
This would likely manifest as a test which generated code and compared the generated output to a known string. We currently don't have any such tests. We could certainly add some (separately), though they would likely be a non-trivial time sink until the code has stabilized further, as many changes would likely require regenerating all such pre-generated snippets. |
When we're outputting code to match a "multi" (a sequence of multiple characters), we're currently issuing a Slice for the known tracked offset even if that offset is 0. We can skip that nop.
cc: @joperezr, @danmoseley
Matching "defg" as part of a pattern before
Matching "defg" as part of a pattern after