Explore regex source generator #7598

rainersigwald · 2022-05-04T16:14:08Z

.NET 7 introduces a regular expression source generator that can generate a bunch of code at compile time to make a more optimal matcher.

Since MSBuild uses many regular expressions and knows a bunch of them at compile time, we should adopt this.

Possible locations:

When validating property/item names (thanks @lambdageek)
in CanonicalError
in ConditionEvaluator
In SolutionFile.cs

(non-exhaustive)

The text was updated successfully, but these errors were encountered:

AraHaan · 2022-05-04T20:58:50Z

It would be nice if the .NET Framework version of msbuild could go away for .NET Core based ones and have Visual Studio use the one from only the .NET SDK so it does not have to bundle a version of msbuild as well to where the .NET Core msbuild can compile both .NET SDK style projects and non-sdk style projects.

This would also help improve the quality of msbuild (and Visual Studio) as a whole.

AR-May · 2022-05-09T12:40:02Z

@rainersigwald How much of perf. win do you expect here?

danmoseley · 2022-05-09T14:27:59Z

It is mostly a startup win.
Note that the build for .NET Framework would still need to use the existing pattern, as alluded above. The generator is only for targeting .NET 7+

rainersigwald · 2022-05-09T14:35:13Z

I wouldn't expect a huge win on anything, but some of the pattern matches are in warm inner loops:

Validating every property/item name (so hundreds/project evaluation + many at execution time).
Scanning every line of ToolTask output.

As Dan mentioned, startup time is reduced and there may be possibilities for inlining/JIT magic.

I wish we could use the generator for our .NET Framework target for unification, but alas.

danmoseley · 2022-05-09T14:42:55Z

Cc @stephentoub fyi

stephentoub · 2022-05-09T14:46:06Z

I wish we could use the generator for our .NET Framework target for unification, but alas.

Yeah, you'll end up wanting to do something like:

#if NET7_0_OR_GREATER
[RegexGenerator(Pattern)]
public static partial Regex AwesomeSauce();
#else
public static Regex AwesomeSauce() => s_regex;
private static readonly Regex s_regex = new Regex(Pattern, RegexOptions.Compiled);
#endif

Forgind · 2022-08-18T18:58:57Z

Just noting this is blocked on #7790

Eli-Black-Work · 2022-09-26T08:51:56Z

Looks like #7790 has been merged, so this is unblocked? 🙂

rainersigwald · 2022-09-26T13:42:14Z

@Bosch-Eli-Black Yes, but to be clear it remains low-priority for the core team.

Eli-Black-Work · 2022-09-27T02:29:33Z

@rainersigwald Understood 🙂

danmoseley · 2022-09-27T03:08:57Z

@Bosch-Eli-Black I expect they'd accept a PR? (and related #7499 too)

Eli-Black-Work · 2022-09-27T03:13:02Z

@danmoseley Haha, ah, yes, I'd love to! 🙂 I've just begun working through my company's procedures for getting authorization to contribute to open source projects, though, and I'm afraid that may take a very long time indeed.

nikijohnson77 · 2022-10-27T20:06:03Z

Do you need any help I don't mind helping so just let me know if you do cuz I just did one of myself last night my project I got done with it last night

Forgind · 2022-10-27T20:16:53Z

@nikijohnson77 We'd love some! Thanks for volunteering; I just assigned it to you. Submit a PR whenever you're ready, and feel free to ask questions.

JaynieBai · 2022-11-30T08:48:17Z

#if NET7_0_OR_GREATER
[RegexGenerator(Pattern)]
public static partial Regex AwesomeSauce();
#else
public static Regex AwesomeSauce() => s_regex;
private static readonly Regex s_regex = new Regex(Pattern, RegexOptions.Compiled);
#endif

Do you have a definition to distinguish different target currently? such as NET7_0_OR_GREATER. @Forgind

ViktorHofer · 2022-11-30T09:10:56Z

NET7_0_OR_GREATER comes via the SDK, so every project that uses the Microsoft.NET.Sdk should have access to it.

AraHaan · 2022-11-30T09:26:27Z

NET7_0_OR_GREATER comes via the SDK, so every project that uses the Microsoft.NET.Sdk should have access to it.

only if the project targets .NET 7 in its list of target frameworks otherwise the maximum defined is NET6_0_OR_GREATER if up to .NET 6 is in it.

rainersigwald · 2022-11-30T14:30:02Z

In the MSBuild codebase we generally try to avoid generic ifdefs like NET7_0_OR_GREATER in favor of specific ones like FEATURE_REGEX_GENERATOR, defined like

msbuild/src/Directory.BeforeCommon.targets

Lines 88 to 95 in ee6f71d

    
           <PropertyGroup Condition="'$(NetCoreBuild)'=='true'"> 
        
             <CompilerToolsDir>$([System.IO.Path]::Combine($(ToolPackagesDir)Microsoft.Net.Compilers, $(CompilerToolsVersion), "tools"))$([System.IO.Path]::DirectorySeparatorChar)</CompilerToolsDir> 
        
             <DefineConstants>$(DefineConstants);FEATURE_ASSEMBLYLOADCONTEXT</DefineConstants> 
        
             <DefineConstants>$(DefineConstants);FEATURE_RUNTIMEINFORMATION</DefineConstants> 
        
             <DefineConstants>$(DefineConstants);USE_MSBUILD_DLL_EXTN</DefineConstants> 
        
             <DefineConstants>$(DefineConstants);WORKAROUND_COREFX_19110</DefineConstants> 
        
             <DefineConstants>$(DefineConstants);FEATURE_SYMLINK_TARGET</DefineConstants> 
        
           </PropertyGroup>

This is helpful because it self-documents why the code is conditionally compiled; while I don't expect it in this case it's conceivable that the regex source generator could someday work on .NET Framework 4.7.2 targets, and if it did adopting it would mean searching for the specific feature, rather than having to look at all net7+ stuff.

stephentoub · 2022-11-30T14:39:19Z

while I don't expect it in this case it's conceivable that the regex source generator could someday work on .NET Framework 4.7.2 targets

Highly unlikely ;-)

rainersigwald · 2022-11-30T14:43:43Z

But if I keep whining about it in this thread . . . 😇

We got most of our benefit from the granular ifdefs in the other direction: there were lots of features that were not in .NET Core 1.0 that got ported later, and the ifdefs let us gradually reunify a bunch of code. I still prefer the granular-ifdef strategy for its "documentation" of why things are conditional.

stephentoub · 2022-11-30T14:45:01Z

But if I keep whining about it in this thread . . . 😇

Your voice will get tired. 😛

AraHaan · 2022-11-30T14:59:14Z

Alternatively, one can't stop them from defining their own version of the .NET 7 attribute for the source generator and install the generator as a nuget package on non-.NET 7 targets, right? If so, that might also be an option to reunify the code.

stephentoub · 2022-11-30T15:01:43Z

Alternatively, one can't stop them from defining their own version of the .NET 7 attribute for the source generator and install the generator as a nuget package on non-.NET 7 targets, right? If so, that might also be an option to reunify the code.

The generator emits code that uses APIs new to .NET 7, including how the generated code plugs into Regex itself.

danmoseley · 2022-11-30T15:39:18Z

If the pattern is relatively straightforward and stable another option is to inspect the generated code and adjust it to make your own hardcoded pattern matcher, no Regex involved.

rainersigwald · 2022-12-06T16:54:31Z

Just noting that I edited the OP to add a new location, where startup time might be a noticeable win: in solution parsing

msbuild/src/Build/Construction/Solution/SolutionFile.cs

Lines 37 to 70 in 01532a9

    
           // An example of a project line looks like this: 
        
           //  Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "ClassLibrary1", "ClassLibrary1\ClassLibrary1.csproj", "{05A5AD00-71B5-4612-AF2F-9EA9121C4111}" 
        
           private static readonly Lazy<Regex> s_crackProjectLine = new Lazy<Regex>( 
        
               () => new Regex 
        
                   ( 
        
                   "^" // Beginning of line 
        
                   + "Project\\(\"(?<PROJECTTYPEGUID>.*)\"\\)" 
        
                   + "\\s*=\\s*" // Any amount of whitespace plus "=" plus any amount of whitespace 
        
                   + "\"(?<PROJECTNAME>.*)\"" 
        
                   + "\\s*,\\s*" // Any amount of whitespace plus "," plus any amount of whitespace 
        
                   + "\"(?<RELATIVEPATH>.*)\"" 
        
                   + "\\s*,\\s*" // Any amount of whitespace plus "," plus any amount of whitespace 
        
                   + "\"(?<PROJECTGUID>.*)\"" 
        
                   + "$", // End-of-line 
        
                   RegexOptions.Compiled 
        
                   ) 
        
               ); 
        
           // An example of a property line looks like this: 
        
           //      AspNetCompiler.VirtualPath = "/webprecompile" 
        
           // Because website projects now include the target framework moniker as 
        
           // one of their properties, <PROPERTYVALUE> may now have '=' in it.  
        
           private static readonly Lazy<Regex> s_crackPropertyLine = new Lazy<Regex>( 
        
               () => new Regex 
        
                   ( 
        
                   "^" // Beginning of line 
        
                   + "(?<PROPERTYNAME>[^=]*)" 
        
                   + "\\s*=\\s*" // Any amount of whitespace plus "=" plus any amount of whitespace 
        
                   + "(?<PROPERTYVALUE>.*)" 
        
                   + "$", // End-of-line 
        
                   RegexOptions.Compiled 
        
                   ) 
        
               );

Since that's one of the first things we do in a dotnet build foo.sln scenario, the compiled-already-ness would likely remove regex compilation from a critical path.

danmoseley · 2022-12-06T17:22:06Z

Reducing JITting is also helpful if Native AOT is ever a goal. (But given MSBuild reflects over assemblies it dynamically discovers, perhaps that would be challenging..)

danmoseley · 2022-12-06T17:22:34Z

@nikijohnson77 how is it going on this?

AraHaan · 2022-12-06T17:57:12Z

Wait why is the regex used for solution parsing, if only there was a library that msbuild could use that uses a much better way of parsing solution files that is just as fast as .NET 7's regex.

danmoseley · 2022-12-06T19:10:05Z

@AraHaan the change to use a source generated regex is quite localized and straightforward. If you're proposing replacing the component, that is best discussed in an issue specific to that (which I'm guessing exists)

danmoseley · 2022-12-22T01:02:20Z

OK, so I played with this and it's quite doable, but a little less than straightforward because of the complexity of MSBuild regexes, and the fact that they are built up from strings (which sometimes themselves are built up from strings).

As an extreme example, take this regex

msbuild/src/Build/Evaluation/Expander.cs

Lines 3040 to 3049 in a6f6699

    
           internal static readonly Lazy<Regex> NonTransformItemMetadataPattern = new Lazy<Regex>( 
        
               () => new Regex 
        
                   ( 
        
                   @"((?<=" + ItemVectorWithTransformLHS + @")" + ItemMetadataSpecification + @"(?!" + 
        
                   ItemVectorWithTransformRHS + @")) | ((?<!" + ItemVectorWithTransformLHS + @")" + 
        
                   ItemMetadataSpecification + @"(?=" + ItemVectorWithTransformRHS + @")) | ((?<!" + 
        
                   ItemVectorWithTransformLHS + @")" + ItemMetadataSpecification + @"(?!" + 
        
                   ItemVectorWithTransformRHS + @"))", 
        
                   RegexOptions.IgnorePatternWhitespace | RegexOptions.ExplicitCapture | RegexOptions.Compiled 
        
                   ));

which includes strings that are themselves built up, eg.,

msbuild/src/Build/Evaluation/Expander.cs

Line 3060 in a6f6699

    
           private const string ItemVectorWithTransformLHS = @"@\(\s*" + ProjectWriter.itemTypeOrMetadataNameSpecification + @"\s*->\s*'[^']*";

The generator can work on this, creating

[GeneratedRegex("((?<=@\\(\\s*[A-Za-z_][A-Za-z_0-9\\-]*\\s*->\\s*'[^']*)%\\(\\s*(?<ITEM_SPECIFICATION>(?<ITEM_TYPE>[A-Za-z_][A-Za-z_0-9\\-]*)\\s*\\.\\s*)?(?<NAME>[A-Za-z_][A-Za-z_0-9\\-]*)\\s*\\)(?![^']*'(\\s*,\\s*'[^']*')?\\s*\\)))|((?<!@\\(\\s*[A-Za-z_][A-Za-z_0-9\\-]*\\s*->\\s*'[^']*)%\\(\\s*(?<ITEM_SPECIFICATION>(?<ITEM_TYPE>[A-Za-z_][A-Za-z_0-9\\-]*)\\s*\\.\\s*)?(?<NAME>[A-Za-z_][A-Za-z_0-9\\-]*)\\s*\\)(?=[^']*'(\\s*,\\s*'[^']*')?\\s*\\)))|((?<!@\\(\\s*[A-Za-z_][A-Za-z_0-9\\-]*\\s*->\\s*'[^']*@)%\\(\\s*(?<ITEM_SPECIFICATION>(?<ITEM_TYPE>[A-Za-z_][A-Za-z_0-9\\-]*)\\s*\\.\\s*)?(?<NAME>[A-Za-z_][A-Za-z_0-9\\-]*)\\s*\\)(?![^']*'(\\s*,\\s*'[^']*')?\\s*\\)))",RegexOptions.ExplicitCapture|RegexOptions.Compiled|RegexOptions.IgnorePatternWhitespace)]

It's probably worth converting to a verbatim string (put a @ at the front and replace \ with ) in order to make them more readable. I opened suggestion dotnet/runtime#79895 for that.

now I have

[GeneratedRegex(@"((?<=@\(\s*[A-Za-z_][A-Za-z_0-9\-]*\s*->\s*'[^']*)%\(\s*(?<ITEM_SPECIFICATION>(?<ITEM_TYPE>[A-Za-z_][A-Za-z_0-9\-]*)\s*\.\s*)?(?<NAME>[A-Za-z_][A-Za-z_0-9\-]*)\s*\)(?![^']*'(\s*,\s*'[^']*')?\s*\)))|((?<!@\(\s*[A-Za-z_][A-Za-z_0-9\-]*\s*->\s*'[^']*)%\(\s*(?<ITEM_SPECIFICATION>(?<ITEM_TYPE>[A-Za-z_][A-Za-z_0-9\-]*)\s*\.\s*)?(?<NAME>[A-Za-z_][A-Za-z_0-9\-]*)\s*\)(?=[^']*'(\s*,\s*'[^']*')?\s*\)))|((?<!@\(\s*[A-Za-z_][A-Za-z_0-9\-]*\s*->\s*'[^']*@)%\(\s*(?<ITEM_SPECIFICATION>(?<ITEM_TYPE>[A-Za-z_][A-Za-z_0-9\-]*)\s*\.\s*)?(?<NAME>[A-Za-z_][A-Za-z_0-9\-]*)\s*\)(?![^']*'(\s*,\s*'[^']*')?\s*\)))",RegexOptions.ExplicitCapture|RegexOptions.Compiled|RegexOptions.IgnorePatternWhitespace)]

The names of the substring variables had been acting as self documentation, so potentially one could manually add those as comments, something like this:

            [GeneratedRegex(@"
    ((?<=@\(\s*[A-Za-z_][A-Za-z_0-9\-]*\s*->\s*'[^']*        # ItemVectorWithTransformLHS
)
    %\(\s* (?<ITEM_SPECIFICATION>(?<ITEM_TYPE>[A-Za-z_][A-Za-z_0-9\-]*)\s*\.\s*)? (?<NAME>[A-Za-z_][A-Za-z_0-9\-]*) \s*\)        # ItemMetadataSpecification
(?!
    [^']*'(\s*,\s*'[^']*')?\s*\)        # ItemVectorWithTransformRHS
)) | ((?<!
    @\(\s*[A-Za-z_][A-Za-z_0-9\-]*\s*->\s*'[^']*        # ItemVectorWithTransformLHS
)
    %\(\s* (?<ITEM_SPECIFICATION>(?<ITEM_TYPE>[A-Za-z_][A-Za-z_0-9\-]*)\s*\.\s*)? (?<NAME>[A-Za-z_][A-Za-z_0-9\-]*) \s*\)        # ItemMetadataSpecification
(?=
    [^']*'(\s*,\s*'[^']*')?\s*\)        # ItemVectorWithTransformRHS
)) | ((?<!
    @\(\s*[A-Za-z_][A-Za-z_0-9\-]*\s*->\s*'[^']*        # ItemVectorWithTransformLHS
@)
    %\(\s* (?<ITEM_SPECIFICATION>(?<ITEM_TYPE>[A-Za-z_][A-Za-z_0-9\-]*)\s*\.\s*)? (?<NAME>[A-Za-z_][A-Za-z_0-9\-]*) \s*\)        # ItemMetadataSpecification
(?!
    [^']*'(\s*,\s*'[^']*')?\s*\)        # ItemVectorWithTransformRHS      
))
", RegexOptions.ExplicitCapture | RegexOptions.Compiled | RegexOptions.IgnorePatternWhitespace)]

TBH I'm not sure that's clearer. I don't think I would bother. It would be better to just memorialize the existing fragments in comments, rather than in the regex.

Note one other wrinkle - a number of these files compile for older versions than .NET 7. So it is necessary to wrap these in #if, so something like

            internal static readonly Lazy<Regex> ItemMetadataPattern = new Lazy<Regex>(
                () => new Regex(ItemMetadataSpecification,
                    RegexOptions.IgnorePatternWhitespace | RegexOptions.ExplicitCapture | RegexOptions.Compiled));

becomes

            internal static readonly Lazy<Regex> ItemMetadataPattern = new Lazy<Regex>(() => 
#if NET7_0_OR_GREATER
                GetItemMetadataPatternRegex());
#else
                new Regex(ItemMetadataSpecification,
                    RegexOptions.IgnorePatternWhitespace | RegexOptions.ExplicitCapture | RegexOptions.Compiled));
#endif

#if NET7_0_OR_GREATER
            [GeneratedRegex("%\\(\\s* (?<ITEM_SPECIFICATION>(?<ITEM_TYPE>[A-Za-z_][A-Za-z_0-9\\-]*)\\s*\\.\\s*)? (?<NAME>[A-Za-z_][A-Za-z_0-9\\-]*) \\s*\\)", RegexOptions.ExplicitCapture | RegexOptions.Compiled | RegexOptions.IgnorePatternWhitespace)]
            private static partial Regex GetItemMetadataPatternRegex();
#endif

These #if's aren't pretty, but it's worth it to eliminate all this JIT work at runtime when running on .NET 7+.

I note that most of the Regexes in the code are constructed lazily, since #1199 to delay generating the IL. I don't think that's necessary for the source generated ones, as constructing them is cheap. However, it's probably simplest to still put them inside the lazy construction, as I've done above, so that all the places that use them can remain unchanged.

danmoseley · 2022-12-22T01:04:46Z

I also hit a couple of issues using the fixer
dotnet/runtime#79891 --- this is partially fixed in .NET 8 builds
dotnet/runtime#79892 --- I dealt with this by just partially reverting the changes.

Neither stop it working, just make a little more manual tuning necessary.

@nikijohnson77 does this make sense? LMK if you need more help.

KalleOlaviNiemitalo · 2022-12-22T07:11:40Z

Doesn't the regex source generator support references to named string constants in the attribute argument?

AraHaan · 2022-12-23T00:07:48Z

I feel like the regex string can be further simplified and made smaller (by defining common parts in each as const and then use string interpolation). So then even in the .NET 7's attribute code, it would use the constant used for the input regex string.

danmoseley · 2022-12-23T03:41:58Z

If that works - great.

danmoseley · 2023-04-19T19:10:30Z

the issues I linked above are now mostly fixed. Is anyone (@AraHaan @KalleOlaviNiemitalo?) interested in doing this? might start with just enabling one or two to see how it goes.

AraHaan · 2023-04-23T09:27:53Z

I been busy lately but I could see what could be done on this when I have more time.

nikijohnson77 · 2023-07-02T20:08:09Z

Ty

nikijohnson77 · 2023-07-13T02:33:52Z

Done

rainersigwald added the performance label May 4, 2022

rainersigwald added this to the .NET 7.0 milestone May 4, 2022

dotnet deleted a comment from AR-May May 9, 2022

AR-May added the backlog label May 16, 2022

Forgind added the blocked label Aug 18, 2022

rainersigwald removed the blocked label Sep 26, 2022

rainersigwald added the help wanted Issues that the core team doesn't plan to work on, but would accept a PR for. Comment to claim. label Sep 27, 2022

Forgind assigned nikijohnson77 Oct 27, 2022

AR-May added the triaged label Feb 21, 2024

JanKrivanek mentioned this issue Jan 3, 2025

changing expander regex #11210

Merged

SimaTian mentioned this issue Jan 6, 2025

replacing a batch of Regex with their generated variant #11228

Merged

Explore regex source generator #7598

Explore regex source generator #7598

Comments

rainersigwald commented May 4, 2022 • edited Loading

AraHaan commented May 4, 2022 • edited Loading

AR-May commented May 9, 2022

danmoseley commented May 9, 2022

rainersigwald commented May 9, 2022

danmoseley commented May 9, 2022

stephentoub commented May 9, 2022 • edited Loading

Forgind commented Aug 18, 2022

Eli-Black-Work commented Sep 26, 2022

rainersigwald commented Sep 26, 2022

Eli-Black-Work commented Sep 27, 2022

danmoseley commented Sep 27, 2022

Eli-Black-Work commented Sep 27, 2022

nikijohnson77 commented Oct 27, 2022

Forgind commented Oct 27, 2022

JaynieBai commented Nov 30, 2022 • edited Loading

ViktorHofer commented Nov 30, 2022

AraHaan commented Nov 30, 2022 • edited Loading

rainersigwald commented Nov 30, 2022

stephentoub commented Nov 30, 2022

rainersigwald commented Nov 30, 2022

stephentoub commented Nov 30, 2022

AraHaan commented Nov 30, 2022 • edited Loading

stephentoub commented Nov 30, 2022

danmoseley commented Nov 30, 2022 • edited Loading

rainersigwald commented Dec 6, 2022

danmoseley commented Dec 6, 2022

danmoseley commented Dec 6, 2022

AraHaan commented Dec 6, 2022

danmoseley commented Dec 6, 2022

danmoseley commented Dec 22, 2022 • edited Loading

danmoseley commented Dec 22, 2022

KalleOlaviNiemitalo commented Dec 22, 2022

AraHaan commented Dec 23, 2022 • edited Loading

danmoseley commented Dec 23, 2022

danmoseley commented Apr 19, 2023

AraHaan commented Apr 23, 2023

nikijohnson77 commented Jul 2, 2023

nikijohnson77 commented Jul 13, 2023

rainersigwald commented May 4, 2022 •

edited

Loading

AraHaan commented May 4, 2022 •

edited

Loading

stephentoub commented May 9, 2022 •

edited

Loading

JaynieBai commented Nov 30, 2022 •

edited

Loading

AraHaan commented Nov 30, 2022 •

edited

Loading

AraHaan commented Nov 30, 2022 •

edited

Loading

danmoseley commented Nov 30, 2022 •

edited

Loading

danmoseley commented Dec 22, 2022 •

edited

Loading

AraHaan commented Dec 23, 2022 •

edited

Loading