Api check tool is slow and cannot handle some projects

[dougbu]

Meant to file aspnet/KoreBuild#207 here:

In the old world, API checks took 40 seconds for a single repo. Haven't measured it yet, but I suspect performance is worse now. (Improvements I imagined did not materialize.)

.NET Framework checks are a bit faster than .NET Core checks because it isn't necessary to parse the .deps.json file or calculate the entire package graph. But, that is not to say these checks are fast. The API check still loads assemblies for all referenced types.

Should investigate two three alternative approaches for loading information about the exposed APIs:

• System.Reflection.Metadata exposes the metadata without needing information from referenced assemblies. The downside is this namespace offers a relatively low-level / hard-to-use surface. See ApiCheck: use System.Reflection.Metadata #198 for some upsides.
• Roslyn Analyzers can examine the API surface as the compiler works. The downside is this approach shifts everything about the API checks. It could still be enabled only in command-line builds and perhaps enabled only conditionally. But, none of the existing hooks would work.
• Mono Cecil

---

dotnet Microsoft.AspNetCore.BuildTools.ApiCheck.dll also crashes when analyzing some projects, usually with FileNotFoundException or BadImageFormatException. These problems will remain after addressing aspnet/KoreBuild#143 due to inherent limitations of using .deps.json files and System.Reflection. Fortunately, these issues affect just one project across our Universe repos: Microsoft.Extensions.Caching.SqlServer in the Caching repo.

The narrow set of the front fell off problems is somewhat minor compared to the speed of the tool.

[natemcmaster]

Thoughts on Roslyn. Disadvantage: requires source code. We lose ability to run api check on binaries. Advantage: VS integration (squiggles in txt editor) and CodeFixes.

Thoughts in s.r.m. Disadvantage: API is really low level. I dont know anyone besides the clr team that knows how to use it. Advantage: its fast.

[davidfowl]

We need to use the fast thing. Hard to use is really a bad excuse. We can also look at mono cecil (it's much friendlier and fast), if it has a .NET Core version (this tool should be .NET Core only since it doesn't need to load anything).

Roslyn would be epic! CODEFIXES for all the things!

[natemcmaster]

I'm sure we can figure out s.r.m. Its not a reason to not do it but its something to be aware of.

Cecil might be a good alternative. We ran into some issues with it in NugetPackageVerifier. #202 Haven't checked lately if newer versions of cecil resolved the errors. This is also something to be aware of, and could be a reason not to use it.

[dougbu]

@natemcmaster are the specific Cecil bugs tracked somewhere? Might be a good time to see which have and haven't been fixed.

[natemcmaster]

For sure this one #202. I'm not aware of the details on others issues we hit in cecil. I vaguely remember hearing it was still buggy but that was about a year ago.

[natemcmaster]

these issues affect a total of 4 projects

Which ones?

[dougbu]

Fortunately, these issues affect a total of 4 projects across our Universe repos.

…

Which ones?

With a fix I made last night, just one project attempts to load an assembly it can't: Microsoft.Extensions.Caching.SqlServer in the Caching repo. That project targets .NET Standard 1.2 but System.Data.SqlClient runtime assemblies are available only for .NET Standard 1.3 and up.

So, this issue should focus on performance and usability, not fragility, at least for now.

I'll update the description…

[javiercn]

Before embarking on exploring any other implementation alternative I would make sure we weight what benefits that would have.

In the old world, API checks took 40 seconds for a single repo

What repository, how many projects are in it, what percentage of the total build time that represents and what the delta is of the build time with the tool vs without it.
Is there something cheap that we can do to improve this? (Maybe run the tool in parallel)

As for running the tool within Visual Studio we decided at the time that this was not a goal for the tool #205 (comment)

I personally think that it would be really annoying if the tool is in my face when I'm doing a breaking change (or preventing my build from succeeding in VS) and if I do a breaking change without noticing its not usually a big issue to revert it and work around it.

So the questions that I would us to solve before doing any of this are:

• Are our needs/goals not fulfilled by the tool as is?
  • What goal/need would a different strategy fulfill?
• Is there a performance problem with the tool when looked on the context of the total build time?
  • Is there a cheaper change that we can do to improve the performance?
  • Do we have any data to back up that a different approach would yield a significant improvement in performance that would justify the investment in rewriting the tool?

/cc @Eilon in case he wants to chime in.

[Eilon]

Currently there are no plans to make any of these changes because the value isn't clear.

[davidfowl]

Is there a cheaper change that we can do to improve the performance?

Do we have existing numbers?

Do we have any data to back up that a different approach would yield a significant improvement in performance that would justify the investment in rewriting the tool?

It would be super easy to validate this if we want numbers.

[Eilon]

Closing because there are no plans to do this. It's not clear what the value is, the cost for the proposed fixes is huge, and the problems it's causing are extremely limited (just one repo, which almost never gets changed).