Roslyn: Compiler server should cache more DLL references

[jaredpar]

The compiler server caches DLLs on the assumption that compilations across a solution are likely to have a large number of identical DLL references. Given that our assembly symbols are immutable caching is safe and can provide significant speed up for builds.

The size of the cache though is presently 100 DLLs. This number was chosen back in the .NET Framework days when the number of references for a project was small. In the more modern .NET Core world this number is significantly higher. An ASP.NET Core app for instance can easily have 300 references hence the cache isn't really serving its intended purpose anymore.

We should update this number to be inline with modern .NET applications.

[jmarolf]

Based on telemetry investigations with @cartermp I believe 90% of projects have less than 300 references. The default ASP.NET web template today starts with a minimum of 283 references (this is the most of any template we ship). 300 seems like it would be better than what we have today, but its unclear to me what the limit is even doing (assuming we are a 64-bit process). Why not set it to 1,000 and let large projects get the 30% perf boost as well? Why not 10,000?

[cartermp]

int.Maxvalue?

[jaredpar]

Why not 10,000?

Because a cache without a policy is just a memory leak. 😄

Seriously at some point it does start to take up a bit of memory. Need to find the balance. My initial stab was to go for ~500.

[cartermp]

Maybe this deserves some experimentation, but I think we should probably bias towards a very high number. Let's say a customer truly had 10k or more assembly references such that it would be a memory consumption issue. Would Visual Studio even be able to handle such a codebase in the first place?

[DamianEdwards]

Using telemetry from June, the curve looks like this for .NET Core projects from Visual Studio:

A few points:

• No project had more than 600 references
• Out of ~8.7M projects, only about 4,000 had more than 500 references
• The spike at "0" is projects reporting no references at all which is very likely a reporting/processing bug so should just be ignored
• Taking the above into consideration, effectively all .NET Core projects had more than 100 references

[jaredpar]

Let's say a customer truly had 10k or more assembly references such that it would be a memory consumption issue.

Customers absolutely do this.

Would Visual Studio even be able to handle such a codebase in the first place?

Yes. Adding this level of caching into the system is likely to make the strain on the system even worse.

[jmarolf]

All I'm saying is that looking at telemetry data biases us toward whatever templates we ship. Most of the projects that are created and reported to telemetry never add or remove _any_ references they are just the default dotnet new template (which is what Damien's bar graph shows). If the goal is just to improve caching in the templates we ship, great! Mission accomplished if we pick 500 (or 300 frankly). If we want to offer more scalability beyond that we may want to consider picking larger numbers or allowing the user to configure this in the same way they can configure the timeout.

[jaredpar]

The idea of allowing this to be configured has come up before. Specifically in cases where there were customers with 5K references and wanted to configure the compiler server such that they could specify that level of caching. We decided to reject it being easily configurable though because at that level of caching you're potentially pushing the server into other problems.

We have to keep in mind that the compiler server is run on a variety of machine types and these have different performance characteristics. Simple changes like altering a timeout to be shorter have caused us to wreck customers deployments because it created some interesting process startup races on their ASP websites. Increasing the cache here will increase the memory pressure of the server and it's possible, or even likely, this will cause similar issues.

At the same time though, the core reason we have the cache is to support builds of our mainline scenarios. The current number of 100 is not achieving that and we need to move to one that will. My preference is to start with 500 because that will encompass the main scenarios with a little room to spare. If this causes issues we can back off.

[jmarolf]

I am in agreement. The obvious thing to do is to change the number from 100 to 500 that should not adversely effect anything (or at least we will learn something new if it does). I am fine with treating customizing caching as out of scope. We'll worry about that when (or if) it comes up.

[CyrusNajmabadi]

A good algorithm here is https://en.wikipedia.org/wiki/Adaptive_replacement_cache

[jmarolf]

@CyrusNajmabadi that does look like an improvement to ensure common assemblies like mscorlib don't get evicted