Improve incremental handling in regex generator #83868
Conversation
- Don't calculate the tree or analysis inside the semantic info. - Dont' store methodDeclartionSyntax and store a comparable across runs location instead.
stephentoub
requested review from
eiriktsarpalis,
CyrusNajmabadi,
joperezr and
chsienki
ghost
assigned 
|
Tagging subscribers to this area: @dotnet/area-system-text-regularexpressions Issue DetailsI noticed that the regex generator wasn't being as incremental as was intended. One issue that had previously been pointed out was that the data returned from the initial pipeline stage included a SyntaxNode, which caused comparison of the cached result against a new compilation to fail. Another issue @chsienki found was that the initial stage was doing the regex parsing and including the parsed regex tree in the result, but that doesn't define anything other than reference equality, so it was also prohibiting reuse of the cached result. On top of that, even if the caching worked, the regex parsing and analysis is relatively expensive, and it would end up being done on every recompilation regardless of whether the result could be cached. Chris fixed the location issue by creating a new Location with the relevant file path/span from the original but without referencing the syntax tree. And he fixed the parsing by moving that logic out into a subsequent stage. We also cleaned up the parsing routine, which no longer needed to find the GeneratedRegexAttribute on its own, as it's now handed into the method in the context supplied by ForAttributeWithMetadataName.
|
src/libraries/System.Text.RegularExpressions/gen/RegexGenerator.Parser.cs
Show resolved
Hide resolved
I noticed that the regex generator wasn't being as incremental as was intended. One issue that had previously been pointed out was that the data returned from the initial pipeline stage included a SyntaxNode, which caused comparison of the cached result against a new compilation to fail. Another issue @chsienki found was that the initial stage was doing the regex parsing and including the parsed regex tree in the result, but that doesn't define anything other than reference equality, so it was also prohibiting reuse of the cached result. On top of that, even if the caching worked, the regex parsing and analysis is relatively expensive, and it would end up being done on every recompilation regardless of whether the result could be cached.
Chris fixed the location issue by creating a new Location with the relevant file path/span from the original but without referencing the syntax tree. And he fixed the parsing by moving that logic out into a subsequent stage.
We also cleaned up the parsing routine, which no longer needed to find the GeneratedRegexAttribute on its own, as it's now handed into the method in the context supplied by ForAttributeWithMetadataName.