-
Notifications
You must be signed in to change notification settings - Fork 1.4k
5% of a build is due ResolveAssemblyReferences's cache checking #2440
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Comments
I assumed computing the closure only occurred when FindDependencies is on, turns out we always compute the closure. The shear number of assemblies coming in, is killing us here. |
Okay, I can see very quickly that we're doing a GetLastWriteFileUtcTime on a given file, 10s and 10s of times. Why, I don't know yet. |
First stack:
|
Second stack:
third stack:
|
Okay I can see why this is happening; we have a cache in SystemState.GetFileState - but we always check the modified date time to see if we throw the cache away, even if we only checked 10ms ago. That is the cost. Our invalidation code is costing 4.9% of a build. |
I'm going to spitball some quick ideas since RAR is by far the biggest time sink in my build right now. I need to set up a workbench to test out ideas for myself but at first glance I see a few options to soften the impact on the file system. Option 1 : Throttle GetFileState cache invalidation to n per second. This option is easy to implement and would cut the IO traffic significantly but has obvious drawbacks. Option 2 : Use FileSystemWatcher(s) The idea is to use a watcher to monitor each file in the cache to see if it changes and then push those updates into the backend of the cache. Advantages :
Drawbacks :
Option 3 : Lock files active in the cache Might be an unacceptable change, but if the state of files involved in the build is static then the cache will always be up to date eliminating the need for the lock. If the build is expected to modify some of the files in the cache, then perhaps inheritable files locks are a solution. |
Sounds good, make note that the build does produce some of the assemblies in the cache - think project-to-project references. |
Before I get started, do you see any issue with locking the cached files? I'm thinking I like option 3 the best at the moment. |
I see no problem if the state does not change over the lifetime of a single RAR call. Over multiple RAR calls, however, will be problematic. |
Changed this bug to be about the cache checking, https://github.com/dotnet/core-setup/issues/3297 is tracking other performance overhead. |
Fixed in: davkean@99d4c83. |
Fixes: dotnet#2440. This makes sure that we only ever call GetLastWriteTime once during a single ResolveAssemblyReference session. Note: I tried to maintain the existing behavior as much as possible except one change - previously, if a file on disk was found to be later than the process-wide cache, the process-wide entry would never get evicted (due to TryAdd). This reduces CPU in GetFileState from 2% -> 0.7% in a mixed .NET Framework/.NET Standard solution.
See: dotnet/sdk#1496.
This is probably going to be all RAR, how do we make this play nicely with incremental?
The text was updated successfully, but these errors were encountered: