From 093e1dd144b260b58a0ae46d722d1dbc4019d9d5 Mon Sep 17 00:00:00 2001 From: Rob Mensching Date: Wed, 6 Jan 2021 15:15:35 -0800 Subject: Implement improved file sequence optimization First ensures files are grouped by DiskId. Then files are sequenced by target directory order to optimize MSI installation behavior. Finally, files are alphabetized in the directory. Additional optimizations could be considered in the future from here. Fixes wixtoolset/issues#4409 Fixes wixtoolset/issues#4708 --- .../Bind/BindDatabaseCommand.cs | 2 +- .../Bind/OptimizeFileFacadesOrderCommand.cs | 92 ++++++++++++++++++++-- src/WixToolset.Core/Linker.cs | 6 +- .../WixToolsetTest.CoreIntegration/CabFixture.cs | 2 +- .../WixToolsetTest.CoreIntegration/MediaFixture.cs | 62 +++++++++++++++ .../TestData/Media/MultiMedia.wxs | 28 +++++++ .../TestData/Media/data/a1.txt | 1 + .../TestData/Media/data/a2.txt | 1 + .../TestData/Media/data/b1.txt | 1 + .../TestData/Media/data/b2.txt | 1 + 10 files changed, 185 insertions(+), 11 deletions(-) create mode 100644 src/test/WixToolsetTest.CoreIntegration/MediaFixture.cs create mode 100644 src/test/WixToolsetTest.CoreIntegration/TestData/Media/MultiMedia.wxs create mode 100644 src/test/WixToolsetTest.CoreIntegration/TestData/Media/data/a1.txt create mode 100644 src/test/WixToolsetTest.CoreIntegration/TestData/Media/data/a2.txt create mode 100644 src/test/WixToolsetTest.CoreIntegration/TestData/Media/data/b1.txt create mode 100644 src/test/WixToolsetTest.CoreIntegration/TestData/Media/data/b2.txt (limited to 'src') diff --git a/src/WixToolset.Core.WindowsInstaller/Bind/BindDatabaseCommand.cs b/src/WixToolset.Core.WindowsInstaller/Bind/BindDatabaseCommand.cs index 93c617d9..4b3d554a 100644 --- a/src/WixToolset.Core.WindowsInstaller/Bind/BindDatabaseCommand.cs +++ b/src/WixToolset.Core.WindowsInstaller/Bind/BindDatabaseCommand.cs @@ -364,7 +364,7 @@ namespace WixToolset.Core.WindowsInstaller.Bind Dictionary> filesByCabinetMedia; IEnumerable uncompressedFiles; { - var order = new OptimizeFileFacadesOrderCommand(fileFacades); + var order = new OptimizeFileFacadesOrderCommand(this.BackendHelper, this.PathResolver, section, platform, fileFacades); order.Execute(); fileFacades = order.FileFacades; diff --git a/src/WixToolset.Core.WindowsInstaller/Bind/OptimizeFileFacadesOrderCommand.cs b/src/WixToolset.Core.WindowsInstaller/Bind/OptimizeFileFacadesOrderCommand.cs index 6943d345..e96dfd91 100644 --- a/src/WixToolset.Core.WindowsInstaller/Bind/OptimizeFileFacadesOrderCommand.cs +++ b/src/WixToolset.Core.WindowsInstaller/Bind/OptimizeFileFacadesOrderCommand.cs @@ -4,34 +4,112 @@ namespace WixToolset.Core.WindowsInstaller.Bind { using System; using System.Collections.Generic; + using System.Linq; using WixToolset.Core.Bind; + using WixToolset.Data; + using WixToolset.Data.Symbols; + using WixToolset.Extensibility.Data; + using WixToolset.Extensibility.Services; internal class OptimizeFileFacadesOrderCommand { - public OptimizeFileFacadesOrderCommand(List fileFacades) + public OptimizeFileFacadesOrderCommand(IBackendHelper helper, IPathResolver pathResolver, IntermediateSection section, Platform platform, List fileFacades) { + this.BackendHelper = helper; + this.PathResolver = pathResolver; + this.Section = section; + this.Platform = platform; this.FileFacades = fileFacades; } public List FileFacades { get; private set; } + private IBackendHelper BackendHelper { get; } + + private IPathResolver PathResolver { get; } + + private IntermediateSection Section { get; } + + private Platform Platform { get; } + public List Execute() { - this.FileFacades.Sort(FileFacadeOptimizer.Instance); + var canonicalComponentTargetPaths = this.ComponentTargetPaths(); + + this.FileFacades.Sort(new FileFacadeOptimizer(canonicalComponentTargetPaths)); return this.FileFacades; } + private Dictionary ComponentTargetPaths() + { + var directories = this.ResolveDirectories(); + + var canonicalPathsByDirectoryId = new Dictionary(); + foreach (var component in this.Section.Symbols.OfType()) + { + var directoryPath = this.PathResolver.GetCanonicalDirectoryPath(directories, null, component.DirectoryRef, this.Platform); + canonicalPathsByDirectoryId.Add(component.Id.Id, directoryPath); + } + + return canonicalPathsByDirectoryId; + } + + private Dictionary ResolveDirectories() + { + var targetPathsByDirectoryId = new Dictionary(); + + // Get the target paths for all directories. + foreach (var directory in this.Section.Symbols.OfType()) + { + var resolvedDirectory = this.BackendHelper.CreateResolvedDirectory(directory.ParentDirectoryRef, directory.Name); + targetPathsByDirectoryId.Add(directory.Id.Id, resolvedDirectory); + } + + return targetPathsByDirectoryId; + } + private class FileFacadeOptimizer : IComparer { - public static readonly FileFacadeOptimizer Instance = new FileFacadeOptimizer(); + public FileFacadeOptimizer(Dictionary componentTargetPaths) + { + this.ComponentTargetPaths = componentTargetPaths; + } + + private Dictionary ComponentTargetPaths { get; } public int Compare(FileFacade x, FileFacade y) { - // TODO: Sort these facades even smarter by directory path and component id - // and maybe file size or file extension and other creative ideas to - // get optimal install speed out of MSI. - return String.Compare(x.ComponentRef, y.ComponentRef, StringComparison.Ordinal); + // First group files by DiskId. + var compare = x.DiskId.CompareTo(y.DiskId); + + if (compare != 0) + { + return compare; + } + + // Next try to group files by target install directory. + if (this.ComponentTargetPaths.TryGetValue(x.ComponentRef, out var canonicalX) && + this.ComponentTargetPaths.TryGetValue(y.ComponentRef, out var canonicalY)) + { + compare = String.Compare(canonicalX, canonicalY, StringComparison.Ordinal); + + if (compare != 0) + { + return compare; + } + } + + // TODO: Consider sorting these facades even smarter by file size or file extension + // or other creative ideas to get optimal install speed out of MSI. + compare = String.Compare(x.FileName, y.FileName, StringComparison.Ordinal); + + if (compare != 0) + { + return compare; + } + + return String.Compare(x.Id, y.Id, StringComparison.Ordinal); } } } diff --git a/src/WixToolset.Core/Linker.cs b/src/WixToolset.Core/Linker.cs index 431ba4c7..e0af89ba 100644 --- a/src/WixToolset.Core/Linker.cs +++ b/src/WixToolset.Core/Linker.cs @@ -198,8 +198,10 @@ namespace WixToolset.Core } // Report duplicates that would ultimately end up being primary key collisions. - var reportDupes = new ReportConflictingSymbolsCommand(this.Messaging, find.PossibleConflicts, resolve.ResolvedSections); - reportDupes.Execute(); + { + var reportDupes = new ReportConflictingSymbolsCommand(this.Messaging, find.PossibleConflicts, resolve.ResolvedSections); + reportDupes.Execute(); + } if (this.Messaging.EncounteredError) { diff --git a/src/test/WixToolsetTest.CoreIntegration/CabFixture.cs b/src/test/WixToolsetTest.CoreIntegration/CabFixture.cs index 5aef148e..ad62dea6 100644 --- a/src/test/WixToolsetTest.CoreIntegration/CabFixture.cs +++ b/src/test/WixToolsetTest.CoreIntegration/CabFixture.cs @@ -42,7 +42,7 @@ namespace WixToolsetTest.CoreIntegration var fileRows = fileTable.Select(r => new FileRow(r)).OrderBy(f => f.Sequence).ToList(); Assert.Equal(new[] { 1, 2 }, fileRows.Select(f => f.Sequence).ToArray()); - Assert.Equal(new[] { "test.txt", "Notepad.exe" }, fileRows.Select(f => f.Name).ToArray()); + Assert.Equal(new[] { "Notepad.exe", "test.txt" }, fileRows.Select(f => f.Name).ToArray()); var files = Query.GetCabinetFiles(cabPath); Assert.Equal(fileRows.Select(f => f.Id).ToArray(), files.Select(f => f.Name).ToArray()); diff --git a/src/test/WixToolsetTest.CoreIntegration/MediaFixture.cs b/src/test/WixToolsetTest.CoreIntegration/MediaFixture.cs new file mode 100644 index 00000000..de18e30c --- /dev/null +++ b/src/test/WixToolsetTest.CoreIntegration/MediaFixture.cs @@ -0,0 +1,62 @@ +// Copyright (c) .NET Foundation and contributors. All rights reserved. Licensed under the Microsoft Reciprocal License. See LICENSE.TXT file in the project root for full license information. + +namespace WixToolsetTest.CoreIntegration +{ + using System.IO; + using System.Linq; + using WixBuildTools.TestSupport; + using WixToolset.Core.TestPackage; + using WixToolset.Data; + using Xunit; + + public class MediaFixture + { + [Fact] + public void CanBuildMultiMedia() + { + var folder = TestData.Get(@"TestData"); + + using (var fs = new DisposableFileSystem()) + { + var baseFolder = fs.GetFolder(); + var intermediateFolder = Path.Combine(baseFolder, "obj"); + var msiPath = Path.Combine(baseFolder, @"bin\test.msi"); + + var result = WixRunner.Execute(new[] + { + "build", + Path.Combine(folder, "Media", "MultiMedia.wxs"), + "-bindpath", Path.Combine(folder, "Media", "data"), + "-intermediateFolder", intermediateFolder, + "-o", msiPath + }); + + result.AssertSuccess(); + + var intermediate = Intermediate.Load(Path.Combine(baseFolder, @"bin\test.wixpdb")); + var section = intermediate.Sections.Single(); + + var mediaSymbols = section.Symbols.OfType().OrderBy(m => m.DiskId).ToList(); + var fileSymbols = section.Symbols.OfType().OrderBy(f => f.Sequence).ToList(); + Assert.Equal(1, mediaSymbols[0].DiskId); + Assert.Equal(2, mediaSymbols[0].LastSequence); + Assert.Equal(2, mediaSymbols[1].DiskId); + Assert.Equal(4, mediaSymbols[1].LastSequence); + Assert.Equal(new[] + { + "a1.txt", + "a2.txt", + "b1.txt", + "b2.txt", + }, fileSymbols.Select(f => f.Name).ToArray()); + Assert.Equal(new[] + { + 1, + 2, + 3, + 4, + }, fileSymbols.Select(f => f.Sequence).ToArray()); + } + } + } +} diff --git a/src/test/WixToolsetTest.CoreIntegration/TestData/Media/MultiMedia.wxs b/src/test/WixToolsetTest.CoreIntegration/TestData/Media/MultiMedia.wxs new file mode 100644 index 00000000..8a555bda --- /dev/null +++ b/src/test/WixToolsetTest.CoreIntegration/TestData/Media/MultiMedia.wxs @@ -0,0 +1,28 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/src/test/WixToolsetTest.CoreIntegration/TestData/Media/data/a1.txt b/src/test/WixToolsetTest.CoreIntegration/TestData/Media/data/a1.txt new file mode 100644 index 00000000..ad9cdcb5 --- /dev/null +++ b/src/test/WixToolsetTest.CoreIntegration/TestData/Media/data/a1.txt @@ -0,0 +1 @@ +This is a1.txt \ No newline at end of file diff --git a/src/test/WixToolsetTest.CoreIntegration/TestData/Media/data/a2.txt b/src/test/WixToolsetTest.CoreIntegration/TestData/Media/data/a2.txt new file mode 100644 index 00000000..d5de23de --- /dev/null +++ b/src/test/WixToolsetTest.CoreIntegration/TestData/Media/data/a2.txt @@ -0,0 +1 @@ +This is a2.txt \ No newline at end of file diff --git a/src/test/WixToolsetTest.CoreIntegration/TestData/Media/data/b1.txt b/src/test/WixToolsetTest.CoreIntegration/TestData/Media/data/b1.txt new file mode 100644 index 00000000..88bc4a56 --- /dev/null +++ b/src/test/WixToolsetTest.CoreIntegration/TestData/Media/data/b1.txt @@ -0,0 +1 @@ +This is b1.txt \ No newline at end of file diff --git a/src/test/WixToolsetTest.CoreIntegration/TestData/Media/data/b2.txt b/src/test/WixToolsetTest.CoreIntegration/TestData/Media/data/b2.txt new file mode 100644 index 00000000..38525276 --- /dev/null +++ b/src/test/WixToolsetTest.CoreIntegration/TestData/Media/data/b2.txt @@ -0,0 +1 @@ +This is b2.txt \ No newline at end of file -- cgit v1.2.3-55-g6feb