[RAR] Stop removing from file cache just because an assembly is cached in process

[Forgind]

Fixes #6844

Context

RAR has several caches, including a process-wide cache and a (serialized) file cache. Previously, we had the process cache take priority (if it was up-to-date) and removed from the serialized cache, believing that meant some other process had taken "ownership" of serializing the assembly, but that doesn't actually make sense because that other process could be the same process currently looking at this assembly, and even if it were another process, that other process could have removed it from its own file cache. This removes that flawed logic so that we always write to the file cache any information we have. This improves incremental build times.

Changes Made

Stopped removing from file cache when process-wide cache has information.

Testing

Tried without this change and noted that the first few attempted cache lookups all missed. Tried with this change without killing processes and noted that they were cache hits; tried with this change after killing processes, and they were still cache hits.

Also noted that without this change, there were 0 things in the instanceLocalFileStateCache at the beginning of the first RAR execution. After it, there were 122.

All tests conducted with Ocelot repo.

Notes

I actually mentioned that I didn't think it made sense to have this code here in the "questions" section of my RAR pre-caching document here but never looked into it more. Ah, well.

[ladipro]

nit: Is this comment still valid?

[Forgind]

One extra thought I had:
With a lot of dlls, serialization cost can be significant. If everything is already in the process cache, this is just wasted time. What would you think of not reading in a state file by default (or giving an option to not read one in?) and only doing so if we find an assembly that isn't in the process cache? Then, if RAR finishes without having read anything in, we don't write anything either.

So ComputeFileStateFromCachesAndDisk would look more like:

Is it in the process cache? Is the process cache up-to-date?
If so, return it.
Else, have we already deserialized an instanceLocalFileStateCache?
If so, check whether it's up-to-date and act accordingly.
Else, deserialize it, check if it has the assembly and is up-to-date and act accordingly.

And we'd remove ReadStateFile and modify WriteStateFile to only write anything if _cache is not null and dirty.

Advantage: Less serialization and deserialization, assuming most of the projects use the same assemblies.
Disadvantage: Less deterministic

Note: May also be a bit less relevant with a RARaaS node if it is only expected to read in a state file cache once per build but would be more relevant if the RARaaS node reads in a state file for every RAR execution.

Thoughts?

[rokonec]

@Forgind I like the idea of lazy load state file once it is needed. The change seems to be simple and safe.