CoreWF icon indicating copy to clipboard operation
CoreWF copied to clipboard

Avoid unnecessary location argument for VB compiling

Open angelowang opened this issue 2 years ago • 3 comments

For CompiledExpressionInvoker.ProcessLocationReferences(), the microsoft reference source is not always creating location argument, but only when RequiresCompilation is true, which is for C#.

https://github.com/microsoft/referencesource/blob/master/System.Activities/System/Activities/Expressions/CompiledExpressionInvoker.cs#L245

This helps for performance a lot when there are a lot of expressions to compile. (21 complex xaml files, the first Execute() drops from 27s to 20s)

angelowang avatar Jul 02 '23 10:07 angelowang

Reference source is for the .NET Framework. In .NET Framework WF, VB expressions are compiled using a native VB expression compiler at runtime. C# expressions have to be compiled with csc through MsBuild so are compiled at build time. CoreWF uses Roslyn, which is the same for both VB and C#. The whole "requirescompilation" flag then becomes obsolete and is only kept for back-compat. Either both VB and C# should have this change or they both should not.

dmetzgar avatar Jul 31 '23 18:07 dmetzgar

Framework: VB - Host Compiler - Runtime compiling - _accessor.CreateLocationArgument(reference, false); not run.

So if I am understanding correctly, in NET6, if they are the same, they both should not call that line? But we still have the check for this:

            if (_textExpression.Language == "VB")
            {
                IList<string> requiredLocationNames = _compiledRoot.GetRequiredLocations(_expressionId);
                CreateRequiredArguments(requiredLocationNames);
            }

Seems already inconsistent.

BTW, CSharpValue code are still similar to .NET framework, and Execute() is using _invoker, instead of some cached _expressionTree.Compile(). That's why I understood they are still different and the requires compilation flag is still somewhat being used.

Our product (using only VB script) removes that and improves performance quite a lot. CoreWF unit tests passing, and our test results also look good :)

angelowang avatar Aug 01 '23 01:08 angelowang

Framework: VB - Host Compiler - Runtime compiling - _accessor.CreateLocationArgument(reference, false); not run.

So if I am understanding correctly, in NET6, if they are the same, they both should not call that line? But we still have the check for this:

            if (_textExpression.Language == "VB")
            {
                IList<string> requiredLocationNames = _compiledRoot.GetRequiredLocations(_expressionId);
                CreateRequiredArguments(requiredLocationNames);
            }

Seems already inconsistent.

BTW, CSharpValue code are still similar to .NET framework, and Execute() is using _invoker, instead of some cached _expressionTree.Compile(). That's why I understood they are still different and the requires compilation flag is still somewhat being used.

Our product (using only VB script) removes that and improves performance quite a lot. CoreWF unit tests passing, and our test results also look good :)

I will have a look at it.

aoltean16 avatar Jan 30 '24 10:01 aoltean16