Allow disabling logging of task parameters and item metadata

[KirillOsenkov]

This enables fine-grained control over whether:

• to log each parameter (whether input or output)
• or whether to log item metadata for each ITaskItem[] parameter.

When LogTaskInputs is set the default behavior is still to log all parameters and all item metadata for ITaskItem[] parameters. Since this is very verbose and hurts performance without adding any useful information it is valuable to be able to turn this logging off in certain situations.

This approach allows controlling logging via setting simple properties or environment variables.

I've identified the specific tasks and parameters that we want to restrict logging for that would give us the most gains without losing any significant useful info:

https://github.com/KirillOsenkov/MSBuildStructuredLog/wiki/Task-Parameter-Logging

[KirillOsenkov]

This is the third alternative to #5235 and #5236.

I believe it's the least invasive and the most flexible and extensible option. One can use plain properties or environment variables defined anywhere and they will be respected if found. This doesn't require as much fundamental changes to the core of MSBuild like the other two approaches. This allows third parties to fully control and override the defaults we provide, either through environment or command line.

I thought about using items instead of properties because the syntax would be nicer:

  <ItemGroup>
    <DisableLogTaskParameter Include="RemoveDuplicates.Inputs" />
    <DisableLogTaskParameterItemMetadata Include="Copy.SourceFiles" />
  </ItemGroup>

However looking up an item would require fetching all items for each parameter, iterating over them to find a match. Items are inherently more expensive, with metadata and whatnot. Also one couldn't use environment variables or pass via command-line /p:foo=bar. Property lookup is faster, lighter weight and can be passed via environment or command-line.

[cdmihai]

Nice, I prefer this approach as well.

Regarding items vs properties, the problem of having to loop through items could be averted by building a lookup dictionary in TaskExecutionHost, but, as you said, one cannot set them from the cmd line. And those lookup dictionaries might get hairy. Since this is a niche feature and not a common use case, I find it okay to use the uglier looking properties. :)

[KirillOsenkov]

I ran this on the Roslyn build and the binlog went down from 130 MB to 120 MB.

I'll see if I can turn off more logging to bring this down even further.

[KirillOsenkov]

The viewer footprint after loading went from 3.7 GB to 2.9 GB. On even larger binlogs such as dotnet/runtime the difference will be even more drastic. I think folks loading large binlogs will appreciate.

[KirillOsenkov]

I've identified a few more candidates to turn down logging of metadata:

• ConvertToAbsolutePath.Paths
• GenerateResource.References
• JoinItems.Left
• JoinItems.Right

After these we quickly hit the diminishing returns. Not sure if I should add these or should we just go with the original PR for now. We can always add stuff later.

[Forgind]

I feel like there has to be a cleaner way of doing this, but I don't know what it is. Makes me wish for Javascript, in which you can just pass a string, and it will automatically convert it to the relevant boolean.

In any case, it would probably be slightly cleaner to have something like:
parameter.Log = !string.Equals(...);

[KirillOsenkov]

If I do the string comparison inside the if, and it matches, it won't evaluate the else if expression.
If I just assign to .Log, both will be always evaluated.
Saving a few microseconds where I can ;)

[KirillOsenkov]

But yes, I feel like there should be an extension method like lookup.IsPropertyTrue(key)

[Forgind]

I'm not sure whether it saves a few microseconds. Your way avoids accessing the property (and I don't know how long that takes). My way avoids branching, which can take a long time relative to just accessing a cached value. I'm guessing you're right, since these methods are rarely simple, and users will probably normally have a lot of properties.

Also, the lookup.IsPropertyTrue method should be easy to make. See:
Forgind@fa2f35c

[Forgind]

I have a very weak preference for putting the other optimizations in a separate PR just because it makes it easier to see what changes are related to what optimizations, but I could be swayed in the other direction, especially because more PRs means more opportunities for flaky failures.

[KirillOsenkov]

Hmm, I feel like this PR is cohesive, meaning there isn't anything you can move out to a separate PR. Unless I'm missing anything?

[Forgind]

Oh, yes, this PR is cohesive. I meant these:
ConvertToAbsolutePath.Paths
GenerateResource.References
JoinItems.Left
JoinItems.Right

[KirillOsenkov]

Ah, got it. Yes, absolutely, I was on the fence about those myself.

I'll just be happy to land this as-is.

[KirillOsenkov]

Roslyn build with this change goes down from 10:01 minutes to 9:45 minutes.