Skip to content
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

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

Merged
merged 3 commits into from
Jun 12, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions src/Tasks.UnitTests/AssemblyDependency/Miscellaneous.cs
Original file line number Diff line number Diff line change
Expand Up @@ -8618,6 +8618,9 @@ public void SDKReferencesAreResolvedWithoutIO()
rar.ResolvedFiles.Length.ShouldBe(1);
rar.ResolvedFiles[0].ItemSpec.ShouldBe(refPath);
rar.ResolvedFiles[0].GetMetadata("FusionName").ShouldBe("System.Candy, Version=8.1.2.3, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a");

// The reference is not worth persisting in the per-instance cache.
rar._cache.IsDirty.ShouldBeFalse();
}

[Fact]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ public void RoundTripEmptyState()

systemState.SerializeCache(_rarCacheFile, _taskLoggingHelper);

var deserialized = SystemState.DeserializeCache(_rarCacheFile, _taskLoggingHelper, typeof(SystemState));
var deserialized = StateFileBase.DeserializeCache<SystemState>(_rarCacheFile, _taskLoggingHelper);

deserialized.ShouldNotBeNull();
}
Expand All @@ -63,7 +63,7 @@ public void CorrectFileVersion()
cacheStream.Close();
}

var deserialized = SystemState.DeserializeCache(_rarCacheFile, _taskLoggingHelper, typeof(SystemState));
var deserialized = StateFileBase.DeserializeCache<SystemState>(_rarCacheFile, _taskLoggingHelper);

deserialized.ShouldNotBeNull();
}
Expand All @@ -81,7 +81,7 @@ public void WrongFileVersion()
cacheStream.Close();
}

var deserialized = SystemState.DeserializeCache(_rarCacheFile, _taskLoggingHelper, typeof(SystemState));
var deserialized = StateFileBase.DeserializeCache<SystemState>(_rarCacheFile, _taskLoggingHelper);

deserialized.ShouldBeNull();
}
Expand All @@ -104,7 +104,7 @@ public void ValidateSerializationAndDeserialization()
{
TransientTestFile file = env.CreateFile();
sysState.SerializeCache(file.Path, null);
sysState2 = SystemState.DeserializeCache(file.Path, null, typeof(SystemState)) as SystemState;
sysState2 = StateFileBase.DeserializeCache<SystemState>(file.Path, null);
}

Dictionary<string, SystemState.FileState> cache2 = sysState2.instanceLocalFileStateCache;
Expand Down
2 changes: 1 addition & 1 deletion src/Tasks.UnitTests/AssemblyRegistrationCache_Tests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ public void ExerciseCacheSerialization()
{
TransientTestFile file = env.CreateFile();
arc.SerializeCache(file.Path, null);
arc2 = StateFileBase.DeserializeCache(file.Path, null, typeof(AssemblyRegistrationCache)) as AssemblyRegistrationCache;
arc2 = StateFileBase.DeserializeCache<AssemblyRegistrationCache>(file.Path, null);
}

arc2._assemblies.Count.ShouldBe(arc._assemblies.Count);
Expand Down
2 changes: 1 addition & 1 deletion src/Tasks.UnitTests/ResolveComReference_Tests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ public void TestSerializationAndDeserialization()
{
TransientTestFile file = env.CreateFile();
cache.SerializeCache(file.Path, null);
cache2 = StateFileBase.DeserializeCache(file.Path, null, typeof(ResolveComReferenceCache)) as ResolveComReferenceCache;
cache2 = StateFileBase.DeserializeCache<ResolveComReferenceCache>(file.Path, null);
}

cache2.tlbImpLocation.ShouldBe(cache.tlbImpLocation);
Expand Down
4 changes: 2 additions & 2 deletions src/Tasks/AssemblyDependency/ResolveAssemblyReference.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2036,7 +2036,7 @@ private void LogConflict(Reference reference, string fusionName, StringBuilder l
/// </summary>
internal void ReadStateFile(FileExists fileExists)
{
_cache = SystemState.DeserializeCache(_stateFile, Log, typeof(SystemState)) as SystemState;
_cache = SystemState.DeserializeCache<SystemState>(_stateFile, Log);

// Construct the cache only if we can't find any caches.
if (_cache == null && AssemblyInformationCachePaths != null && AssemblyInformationCachePaths.Length > 0)
Expand Down Expand Up @@ -2313,7 +2313,7 @@ internal bool Execute(
{
// We don't want to perform I/O to see what the actual timestamp on disk is so we return a fixed made up value.
// Note that this value makes the file exist per the check in SystemState.FileTimestampIndicatesFileExists.
return DateTime.MaxValue;
return SystemState.FileState.ImmutableFileLastModifiedMarker;
}
return getLastWriteTime(path);
});
Expand Down
2 changes: 1 addition & 1 deletion src/Tasks/RegisterAssembly.cs
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ public override bool Execute()

if ((AssemblyListFile?.ItemSpec.Length > 0))
{
cacheFile = (AssemblyRegistrationCache)StateFileBase.DeserializeCache(AssemblyListFile.ItemSpec, Log, typeof(AssemblyRegistrationCache)) ??
cacheFile = StateFileBase.DeserializeCache<AssemblyRegistrationCache>(AssemblyListFile.ItemSpec, Log) ??
new AssemblyRegistrationCache();
}

Expand Down
2 changes: 1 addition & 1 deletion src/Tasks/ResGenDependencies.cs
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,7 @@ internal override void SerializeCache(string stateFile, TaskLoggingHelper log)
/// </summary>
internal static ResGenDependencies DeserializeCache(string stateFile, bool useSourcePath, TaskLoggingHelper log)
{
var retVal = (ResGenDependencies)DeserializeCache(stateFile, log, typeof(ResGenDependencies)) ?? new ResGenDependencies();
var retVal = DeserializeCache<ResGenDependencies>(stateFile, log) ?? new ResGenDependencies();

// Ensure that the cache is properly initialized with respect to how resgen will
// resolve linked files within .resx files. ResGen has two different
Expand Down
2 changes: 1 addition & 1 deletion src/Tasks/ResolveComReference.cs
Original file line number Diff line number Diff line change
Expand Up @@ -333,7 +333,7 @@ public override bool Execute()
allProjectRefs = new List<ComReferenceInfo>();
allDependencyRefs = new List<ComReferenceInfo>();

_timestampCache = (ResolveComReferenceCache)StateFileBase.DeserializeCache(StateFile, Log, typeof(ResolveComReferenceCache));
_timestampCache = StateFileBase.DeserializeCache<ResolveComReferenceCache>(StateFile, Log);

if (_timestampCache?.ToolPathsMatchCachePaths(_tlbimpPath, _aximpPath) != true)
{
Expand Down
11 changes: 5 additions & 6 deletions src/Tasks/StateFileBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -67,9 +67,9 @@ internal virtual void SerializeCache(string stateFile, TaskLoggingHelper log)
/// <summary>
/// Reads the specified file from disk into a StateFileBase derived object.
/// </summary>
internal static StateFileBase DeserializeCache(string stateFile, TaskLoggingHelper log, Type requiredReturnType)
internal static T DeserializeCache<T>(string stateFile, TaskLoggingHelper log) where T : StateFileBase
{
StateFileBase retVal = null;
T retVal = null;

// First, we read the cache from disk if one exists, or if one does not exist, we create one.
try
Expand All @@ -90,21 +90,20 @@ internal static StateFileBase DeserializeCache(string stateFile, TaskLoggingHelp
return null;
}

var constructors = requiredReturnType.GetConstructors();
var constructors = typeof(T).GetConstructors();
foreach (var constructor in constructors)
{
var parameters = constructor.GetParameters();
if (parameters.Length == 1 && parameters[0].ParameterType == typeof(ITranslator))
{
retVal = constructor.Invoke(new object[] { translator }) as StateFileBase;
retVal = constructor.Invoke(new object[] { translator }) as T;
}
}

if (retVal == null || !requiredReturnType.IsInstanceOfType(retVal))
if (retVal == null)
{
log.LogMessageFromResources("General.CouldNotReadStateFileMessage", stateFile,
log.FormatResourceString("General.IncompatibleStateFileType"));
retVal = null;
}
}
}
Expand Down
45 changes: 35 additions & 10 deletions src/Tasks/SystemState.cs
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,16 @@ internal FrameworkName FrameworkNameAttribute
get { return frameworkName; }
set { frameworkName = value; }
}

/// <summary>
/// The last-modified value to use for immutable framework files which we don't do I/O on.
/// </summary>
internal static DateTime ImmutableFileLastModifiedMarker => DateTime.MaxValue;

/// <summary>
/// It is wasteful to persist entries for immutable framework files.
/// </summary>
internal bool IsWorthPersisting => lastModified != ImmutableFileLastModifiedMarker;
ladipro marked this conversation as resolved.
Show resolved Hide resolved
}

/// <summary>
Expand Down Expand Up @@ -258,7 +268,7 @@ public override void Translate(ITranslator translator)
}

/// <summary>
/// Flag that indicates
/// Flag that indicates that <see cref="instanceLocalFileStateCache"/> has been modified.
/// </summary>
/// <value></value>
internal bool IsDirty
Expand Down Expand Up @@ -339,7 +349,7 @@ private FileState GetFileState(string path)
{
// Looking up an assembly to get its metadata can be expensive for projects that reference large amounts
// of assemblies. To avoid that expense, we remember and serialize this information betweeen runs in
// XXXResolveAssemblyReferencesInput.cache files in the intermediate directory and also store it in an
// <ProjectFileName>.AssemblyReference.cache files in the intermediate directory and also store it in an
// process-wide cache to share between successive builds.
//
// To determine if this information is up-to-date, we use the last modified date of the assembly, however,
Expand Down Expand Up @@ -368,8 +378,8 @@ private FileState ComputeFileStateFromCachesAndDisk(string path)
// If the process-wide cache contains an up-to-date FileState, always use it
if (isProcessFileStateUpToDate)
{
// For the next build, we may be using a different process. Update the file cache.
if (!isInstanceFileStateUpToDate)
// For the next build, we may be using a different process. Update the file cache if the entry is worth persisting.
if (!isInstanceFileStateUpToDate && cachedProcessFileState.IsWorthPersisting)
{
instanceLocalFileStateCache[path] = cachedProcessFileState;
isDirty = true;
Expand Down Expand Up @@ -399,9 +409,15 @@ private DateTime GetAndCacheLastModified(string path)
private FileState InitializeFileState(string path, DateTime lastModified)
{
var fileState = new FileState(lastModified);
instanceLocalFileStateCache[path] = fileState;

// Dirty the instance-local cache only with entries that are worth persisting.
if (fileState.IsWorthPersisting)
{
instanceLocalFileStateCache[path] = fileState;
isDirty = true;
}

s_processWideFileStateCache[path] = fileState;
isDirty = true;

return fileState;
}
Expand Down Expand Up @@ -450,7 +466,10 @@ private AssemblyNameExtension GetAssemblyName(string path)
{
fileState.Assembly = AssemblyNameExtension.UnnamedAssembly;
}
isDirty = true;
if (fileState.IsWorthPersisting)
{
isDirty = true;
}
}

if (fileState.Assembly.IsUnnamedAssembly)
Expand All @@ -471,7 +490,10 @@ private string GetRuntimeVersion(string path)
if (String.IsNullOrEmpty(fileState.RuntimeVersion))
{
fileState.RuntimeVersion = getAssemblyRuntimeVersion(path);
isDirty = true;
if (fileState.IsWorthPersisting)
{
isDirty = true;
}
}

return fileState.RuntimeVersion;
Expand Down Expand Up @@ -503,7 +525,10 @@ private void GetAssemblyMetadata(
out fileState.scatterFiles,
out fileState.frameworkName);

isDirty = true;
if (fileState.IsWorthPersisting)
{
isDirty = true;
}
}

dependencies = fileState.dependencies;
Expand All @@ -527,7 +552,7 @@ internal static SystemState DeserializePrecomputedCaches(ITaskItem[] stateFiles,
foreach (ITaskItem stateFile in stateFiles)
{
// Verify that it's a real stateFile. Log message but do not error if not.
SystemState sysState = DeserializeCache(stateFile.ToString(), log, typeof(SystemState)) as SystemState;
SystemState sysState = DeserializeCache<SystemState>(stateFile.ToString(), log);
if (sysState == null)
{
continue;
Expand Down
2 changes: 1 addition & 1 deletion src/Tasks/UnregisterAssembly.cs
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ public override bool Execute()

if (AssemblyListFile != null)
{
cacheFile = (AssemblyRegistrationCache)StateFileBase.DeserializeCache(AssemblyListFile.ItemSpec, Log, typeof(AssemblyRegistrationCache));
cacheFile = StateFileBase.DeserializeCache<AssemblyRegistrationCache>(AssemblyListFile.ItemSpec, Log);

// no cache file, nothing to do. In case there was a problem reading the cache file, we can't do anything anyway.
if (cacheFile == null)
Expand Down