Skip to content

Commit

Permalink
Properly rebuild optimization data when it changes (#56397)
Browse files Browse the repository at this point in the history
The timestamp of the merged .mibc file was set to when the tool was
invoked, while the inputs have a timestamp from when they were created
in the training scenarios. That means the target to create the merged
.mibc file would not run incrementally until many weeks later.

To fix add an --inherit-timestamp flag to dotnet-pgo that makes the
merged output inherit the max timestamp from the inputs. This is not
ideal as it means incrementally going backwards does not work, but it's
better than the previous behavior.

Fix #53637
  • Loading branch information
jakobbotsch committed Jul 28, 2021
1 parent 504f1e8 commit cf52b7e
Show file tree
Hide file tree
Showing 3 changed files with 12 additions and 1 deletion.
1 change: 1 addition & 0 deletions src/coreclr/crossgen-corelib.proj
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@
<DotNetPgoCmd>$(DotNetCli) $([MSBuild]::NormalizePath('$(BinDir)', 'dotnet-pgo', 'dotnet-pgo.dll')) merge</DotNetPgoCmd>
<DotNetPgoCmd>$(DotNetPgoCmd) -o:$(MergedMibcPath)</DotNetPgoCmd>
<DotNetPgoCmd>$(DotNetPgoCmd) @(OptimizationMibcFiles->'-i:%(Identity)', ' ')</DotNetPgoCmd>
<DotNetPgoCmd>$(DotNetPgoCmd) --inherit-timestamp</DotNetPgoCmd> <!-- For incremental builds, otherwise timestamp is too far in the future -->
</PropertyGroup>

<Message Condition="'$(DotNetBuildFromSource)' != 'true'" Importance="High" Text="$(DotNetPgoCmd)"/>
Expand Down
3 changes: 3 additions & 0 deletions src/coreclr/tools/dotnet-pgo/CommandLineOptions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ internal class CommandLineOptions
public bool DumpMibc = false;
public FileInfo InputFileToDump;
public List<FileInfo> CompareMibc;
public bool InheritTimestamp;

public string[] HelpArgs = Array.Empty<string>();

Expand Down Expand Up @@ -265,6 +266,8 @@ void HelpOption()
}
}

syntax.DefineOption(name: "inherit-timestamp", value: ref InheritTimestamp, help: "If specified, set the output's timestamp to the max timestamp of the input files");

VerbosityOption();
CompressedOption();
HelpOption();
Expand Down
9 changes: 8 additions & 1 deletion src/coreclr/tools/dotnet-pgo/Program.cs
Original file line number Diff line number Diff line change
Expand Up @@ -378,7 +378,14 @@ static int InnerMergeMain(CommandLineOptions commandLineOptions)
ProfileData.MergeProfileData(ref partialNgen, mergedProfileData, MIbcProfileParser.ParseMIbcFile(tsc, peReader, assemblyNamesInBubble, onlyDefinedInAssembly: null));
}

return MibcEmitter.GenerateMibcFile(tsc, commandLineOptions.OutputFileName, mergedProfileData.Values, commandLineOptions.ValidateOutputFile, commandLineOptions.Uncompressed);
int result = MibcEmitter.GenerateMibcFile(tsc, commandLineOptions.OutputFileName, mergedProfileData.Values, commandLineOptions.ValidateOutputFile, commandLineOptions.Uncompressed);
if (result == 0 && commandLineOptions.InheritTimestamp)
{
commandLineOptions.OutputFileName.CreationTimeUtc = commandLineOptions.InputFilesToMerge.Max(fi => fi.CreationTimeUtc);
commandLineOptions.OutputFileName.LastWriteTimeUtc = commandLineOptions.InputFilesToMerge.Max(fi => fi.LastWriteTimeUtc);
}

return result;
}
finally
{
Expand Down

0 comments on commit cf52b7e

Please sign in to comment.