Skip to content

RAR does not write to cache file on rebuilds#2692

Merged
cdmihai merged 3 commits intodotnet:vs15.5from
cdmihai:fixRARCache
Nov 6, 2017
Merged

RAR does not write to cache file on rebuilds#2692
cdmihai merged 3 commits intodotnet:vs15.5from
cdmihai:fixRARCache

Conversation

@cdmihai
Copy link
Copy Markdown
Contributor

@cdmihai cdmihai commented Nov 2, 2017

Closes #2687

Cause of the issue:

  • RAR uses two caches: a process wide cache and a project specific cache
  • The project specific cache is serialized and reused between processes, whereas the process wide cache is not serialized. The process wide cache aggregates the project specific caches as projects are built.
  • On every RAR execution, the project specific cache is loaded from disk, and compared against the value process wide cache: https://github.com/Microsoft/msbuild/blob/99d4c838624293725bfbfd1ae75405ec1bb99c4a/src/Tasks/SystemState.cs#L343-L351
  • since the project wide cache is deserialized each time, its entries' are different instances from the entries in the process wide cache. This makes the reference equality check fail, and leads to dirtying the cache.

Fix: I chose to reuse the old code, wrap it with a guarding collection (to ensure an entry is checked for staleness only once), and ported over the change to replace stale entries from the process wide cache. Did this because the old code had an extra optimization: if the process wide cache had a more recent entry than the project specific cache, it would update the project specific cache from the process cache.

Before / After memory

image

Before / After CPU
image

Using TryAdd means that the first value that gets into the cache does not ever get overwritten, even when the data is stale.
@cdmihai
Copy link
Copy Markdown
Contributor Author

cdmihai commented Nov 2, 2017

@davkean

@davkean
Copy link
Copy Markdown
Member

davkean commented Nov 2, 2017

I likely regressed this.

@cdmihai
Copy link
Copy Markdown
Contributor Author

cdmihai commented Nov 3, 2017

Nothing interesting here. Build time suggests positive improvement, as every case seems to be an improvement, with largest in the mvc project which is RAR heavy.

Evaluation: Time (ms)

Test Overall Significant δ Value
DotnetConsoleProject yes 95.1051 -> 94.1112 (-1.045%)
DotnetWebProject 🔴 yes 137.2171 -> 137.8964 (0.495%)
DotnetMvcProject yes 163.5339 -> 160.2404 (-2.014%)
Picasso yes 778.1822 -> 757.3825 (-2.673%)
SmallP2POldCsproj yes 135.6276 -> 132.9834 (-1.95%)
SmallP2PNewCsproj yes 244.8956 -> 242.6864 (-0.902%)
Generated_100_100_v150 👌 no 1595.2215 -> 1603.8711 (0.542%)
LargeP2POldCsproj ::ok_hand: no 2245.3647 -> 2243.9941 (-0.061%)
roslyn 🔴 yes 9342.4617 -> 9518.1423 (1.88%)

Evaluation: Memory (bytes)

Test Overall Significant δ Value
DotnetConsoleProject ::ok_hand: no 5092600 -> 5091897 (-0.014%)
DotnetWebProject ::ok_hand: no 6767956 -> 6767334 (-0.009%)
DotnetMvcProject ::ok_hand: no 7612326 -> 7611534 (-0.01%)
Picasso ::ok_hand: no 33896648 -> 33896578 (0%)
SmallP2POldCsproj 👌 no 6107778 -> 6107937 (0.003%)
SmallP2PNewCsproj ::ok_hand: no 10552742 -> 10552735 (0%)
Generated_100_100_v150 ::ok_hand: no 140411739 -> 140411333 (0%)
LargeP2POldCsproj 👌 no 83129678 -> 83130926 (0.002%)
roslyn ::ok_hand: no 398736024 -> 397053171 (-0.422%)

Build: Time (ms)

Test Overall Significant δ Value
DotnetConsoleProject ::ok_hand: no 23.6024 -> 23.497 (-0.447%)
DotnetWebProject 👌 no 28.2513 -> 28.2599 (0.03%)
DotnetMvcProject yes 31.5754 -> 30.8451 (-2.313%)
Picasso yes 4584.9235 -> 4537.6154 (-1.032%)
SmallP2POldCsproj yes 199.8064 -> 198.7935 (-0.507%)
SmallP2PNewCsproj yes 176.9933 -> 175.8262 (-0.659%)

Build: Memory (bytes)

Test Overall Significant δ Value
DotnetConsoleProject yes 1166317 -> 1165472 (-0.072%)
DotnetWebProject 🔴 yes 1310784 -> 1312840 (0.157%)
DotnetMvcProject 👌 no 1484924 -> 1485014 (0.006%)
Picasso 👌 no 194598709 -> 195218981 (0.319%)
SmallP2POldCsproj yes 8185243 -> 8179606 (-0.069%)
SmallP2PNewCsproj 👌 no 7256183 -> 7259937 (0.052%)

Copy link
Copy Markdown
Contributor

@AndyGerlicher AndyGerlicher left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved, @cdmihai merge it please.

@cdmihai cdmihai merged commit 57dd2b3 into dotnet:vs15.5 Nov 6, 2017
@cdmihai cdmihai deleted the fixRARCache branch April 26, 2018 17:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants