Skip to content

Don't save data on immutable files to per-project disk cache - #8802

Merged
JaynieBai merged 3 commits into
dotnet:mainfrom
ladipro:smaller-rar-cache-file
Jun 12, 2023
Merged

JaynieBai merged 3 commits into
dotnet:mainfrom
ladipro:smaller-rar-cache-file

Conversation

@ladipro

@ladipro ladipro commented May 26, 2023

Copy link
Copy Markdown
Member

Contributes to #8635

Context

With #8688 we no longer need to cache data on immutable framework files on disk as obtaining this information is cheap and does not require I/O.

Changes Made

Made SystemState (the RAR cache) not add such files to the per-instance dictionary. This dictionary is serialized to disk as <project-file>.AssemblyReference.cache so the change effectively makes the cache file smaller, saving on serialization and deserialization time.

Also refactored DeserializeCache into a generic method instead of taking a Type parameter.

For a simple ASP.NET Core app with one project reference and one package reference, the size of the cache file is reduced from 142 kB to 2 kB. Projects with no references other than the SDK will not have the cache file created at all.

Testing

Existing unit tests, manual measurements.

Notes

The actual "Don't read the file if the data is already in memory" change will be in a separate upcoming PR.

Comment thread src/Tasks/SystemState.cs

@JanKrivanek JanKrivanek left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good!

@ladipro

ladipro commented Jun 6, 2023

Copy link
Copy Markdown
Member Author

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@JaynieBai
JaynieBai merged commit df97a4b into dotnet:main Jun 12, 2023
@ladipro
ladipro deleted the smaller-rar-cache-file branch June 13, 2023 12:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants