Skip to content

Commit 296a53d

Browse files
JohnMcPMSJohnMcPMS
authored
Performance improvements (microsoft#3808)
## Change This change has several performance improvements based on profiling. ### Installed package caching A global process cache is stored for the unfiltered install data. It is very basic for updates right now to make it more reasonable to service, but future changes could improve things to examine the system data and only do delta updates. It could also be saved to disk to improve the performance of future processes. The cache itself is not used by any callers, but instead in-memory copies are handed out. The cache sets up event listeners for the current data sources and waits for these events before it will attempt to update again. A full cache hit requires only copying the memory database, which SQLite has an efficient method for. The only performance gain to the update path that is provided by this implementation is to reuse the names of MSIX packages, but since this accounts for a huge amount of creating the index it results in ~50% time reduction in creating an updated installed source (without any MSIX changes). ### ICU regex caching The regular expressions that we use were being compiled repeatedly. Since this is a fixed set of expressions, they are all now cached and copies are used for actual operation. ### Version lookup (string) The version lookup by string (such as `winget show FOO -v VER`) was doing a full table scan. This change shifts to pulling all of the version for the given identifier out and using `Version` object comparisons to find the proper result. This has the side effect of fixing a somewhat annoying user interaction where trailing 0s in a version were still required to find from the command line. One can now provide the smaller version string and still find the expected package version (eg `1.2` will now find version `1.2.0`). ### Version lookup (available version) The round trip from retrieving the available version information to getting the manifest has been improved by including the manifest id value directly in the available version data. The available version package object now caches the version string to manifest id data as well, meaning that any usage of a `PackageVersionKey` that was retrieved from `GetAvailableVersions` will directly return the package version object without querying the index. ### Let property lookups handle an unknown manifest id The `GetProperty` code was enforcing that the manifest id was valid and then reusing existing code that did not handle a missing manifest. Creating new methods that can handle the missing manifest allows us to remove the initial check.
1 parent b96598a commit 296a53d

43 files changed

Lines changed: 1006 additions & 236 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎.github/actions/spelling/expect.txt‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,7 @@ ENDSESSION
131131
EPester
132132
epth
133133
EQU
134+
errcode
134135
errmsg
135136
ERRORONEXIT
136137
ESource

‎azure-pipelines.yml‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -123,9 +123,11 @@ jobs:
123123
/p:UapAppxPackageBuildMode=SideloadOnly'
124124

125125
- task: CopyFiles@2
126-
displayName: 'Copy WindowsPackageManager.dll Symbols to artifacts folder'
126+
displayName: 'Copy Symbols to artifacts folder'
127127
inputs:
128-
Contents: '$(buildOutDir)\WindowsPackageManager\WindowsPackageManager.pdb'
128+
Contents: |
129+
$(buildOutDir)\WindowsPackageManager\WindowsPackageManager.pdb
130+
$(buildOutDir)\Microsoft.Management.Configuration\Microsoft.Management.Configuration.pdb
129131
TargetFolder: '$(artifactsDir)'
130132
condition: succeededOrFailed()
131133

@@ -306,7 +308,7 @@ jobs:
306308
platform: '$(buildPlatform)'
307309
configuration: '$(BuildConfiguration)'
308310
diagnosticsEnabled: true
309-
condition: succeededOrFailed()
311+
condition: and(succeededOrFailed(), eq(variables.buildPlatform, 'x86'))
310312

311313
- task: PowerShell@2
312314
displayName: Prepare for Microsoft.Management.Configuration.UnitTests (OutOfProc)

‎src/AppInstallerCLICore/Workflows/InstallFlow.cpp‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
#include "WorkflowBase.h"
1717
#include "DependenciesFlow.h"
1818
#include "PromptFlow.h"
19+
#include "SourceFlow.h"
1920
#include <AppInstallerMsixInfo.h>
2021
#include <AppInstallerDeployment.h>
2122
#include <AppInstallerSynchronization.h>
@@ -553,6 +554,7 @@ namespace AppInstaller::CLI::Workflow
553554
Workflow::ReportExecutionStage(ExecutionStage::PostExecution) <<
554555
Workflow::ReportARPChanges <<
555556
Workflow::RecordInstall <<
557+
Workflow::ForceInstalledCacheUpdate <<
556558
Workflow::RemoveInstaller <<
557559
Workflow::DisplayInstallationNotes;
558560
}

‎src/AppInstallerCLICore/Workflows/SourceFlow.cpp‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -301,4 +301,10 @@ namespace AppInstaller::CLI::Workflow
301301
}
302302
}
303303
}
304+
305+
void ForceInstalledCacheUpdate(Execution::Context&)
306+
{
307+
// Creating this object is currently sufficient to mark the cache as needing an update for the next time it is opened.
308+
Repository::Source ignore{ Repository::PredefinedSource::InstalledForceCacheUpdate };
309+
}
304310
}

‎src/AppInstallerCLICore/Workflows/SourceFlow.h‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,4 +77,10 @@ namespace AppInstaller::CLI::Workflow
7777
// Inputs: SourceList
7878
// Outputs: None
7979
void ExportSourceList(Execution::Context& context);
80+
81+
// Forces an update to the cache of installed packages.
82+
// Required Args: None
83+
// Inputs: None
84+
// Outputs: None
85+
void ForceInstalledCacheUpdate(Execution::Context& context);
8086
}

‎src/AppInstallerCLIE2ETests/Constants.cs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,7 @@ public class Constants
8282
public const string PortableExePackageId = "AppInstallerTest.TestPortableExe";
8383
public const string PortableExeWithCommandPackageId = "AppInstallerTest.TestPortableExeWithCommand";
8484

85+
public const string ExeInstalledDefaultProductCode = "{A499DD5E-8DC5-4AD2-911A-BCD0263295E9}";
8586
public const string MsiInstallerProductCode = "{A5D36CF1-1993-4F63-BFB4-3ACD910D36A1}";
8687
public const string MsixInstallerName = "6c6338fe-41b7-46ca-8ba6-b5ad5312bb0e";
8788
public const string MsixInstallerPackageFamilyName = "6c6338fe-41b7-46ca-8ba6-b5ad5312bb0e_8wekyb3d8bbwe";

‎src/AppInstallerCLIE2ETests/Interop/InstallInterop.cs‎

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -657,5 +657,72 @@ public void GetApplicableInstaller()
657657
Assert.AreEqual(PackageInstallerScope.User, packageInstallerInfo.Scope);
658658
Assert.AreEqual("en-US", packageInstallerInfo.Locale);
659659
}
660+
661+
/// <summary>
662+
/// Install exe and verify that we can find it as installed after.
663+
/// </summary>
664+
/// <returns>A <see cref="Task"/> representing the asynchronous unit test.</returns>
665+
[Test]
666+
public async Task InstallExe_VerifyInstalledCatalog()
667+
{
668+
var installedCatalogReference = this.packageManager.GetLocalPackageCatalog(LocalPackageCatalog.InstalledPackages);
669+
670+
// Ensure package is not installed
671+
var installedResult = this.FindAllPackages(installedCatalogReference, PackageMatchField.ProductCode, PackageFieldMatchOption.Equals, Constants.ExeInstalledDefaultProductCode);
672+
Assert.IsNotNull(installedResult);
673+
Assert.AreEqual(0, installedResult.Count);
674+
675+
// Find package
676+
var searchResult = this.FindOnePackage(this.testSource, PackageMatchField.Id, PackageFieldMatchOption.Equals, "AppInstallerTest.TestExeInstaller");
677+
678+
// Configure installation
679+
var installOptions = this.TestFactory.CreateInstallOptions();
680+
installOptions.PackageInstallMode = PackageInstallMode.Silent;
681+
installOptions.PreferredInstallLocation = this.installDir;
682+
installOptions.AcceptPackageAgreements = true;
683+
684+
// Install
685+
var installResult = await this.packageManager.InstallPackageAsync(searchResult.CatalogPackage, installOptions);
686+
687+
// Assert
688+
Assert.AreEqual(InstallResultStatus.Ok, installResult.Status);
689+
690+
// Check installed catalog after
691+
this.FindOnePackage(installedCatalogReference, PackageMatchField.ProductCode, PackageFieldMatchOption.Equals, Constants.ExeInstalledDefaultProductCode);
692+
693+
Assert.True(TestCommon.VerifyTestExeInstalledAndCleanup(this.installDir));
694+
}
695+
696+
/// <summary>
697+
/// Install msix and verify that we can find it as installed after.
698+
/// </summary>
699+
/// <returns>A <see cref="Task"/> representing the asynchronous unit test.</returns>
700+
[Test]
701+
public async Task InstallMSIX_VerifyInstalledCatalog()
702+
{
703+
var installedCatalogReference = this.packageManager.GetLocalPackageCatalog(LocalPackageCatalog.InstalledPackages);
704+
705+
// Ensure package is not installed
706+
var installedResult = this.FindAllPackages(installedCatalogReference, PackageMatchField.PackageFamilyName, PackageFieldMatchOption.Equals, Constants.MsixInstallerPackageFamilyName);
707+
Assert.IsNotNull(installedResult);
708+
Assert.AreEqual(0, installedResult.Count);
709+
710+
// Find package
711+
var searchResult = this.FindOnePackage(this.testSource, PackageMatchField.Name, PackageFieldMatchOption.Equals, "TestMsixInstaller");
712+
713+
// Configure installation
714+
var installOptions = this.TestFactory.CreateInstallOptions();
715+
716+
// Install
717+
var installResult = await this.packageManager.InstallPackageAsync(searchResult.CatalogPackage, installOptions);
718+
719+
// Assert
720+
Assert.AreEqual(InstallResultStatus.Ok, installResult.Status);
721+
722+
// Check installed catalog after
723+
this.FindOnePackage(installedCatalogReference, PackageMatchField.PackageFamilyName, PackageFieldMatchOption.Equals, Constants.MsixInstallerPackageFamilyName);
724+
725+
Assert.True(TestCommon.VerifyTestMsixInstalledAndCleanup());
726+
}
660727
}
661728
}

‎src/AppInstallerCLITests/AppInstallerCLITests.vcxproj‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -197,6 +197,7 @@
197197
<ClCompile Include="Archive.cpp" />
198198
<ClCompile Include="Argument.cpp" />
199199
<ClCompile Include="ARPChanges.cpp" />
200+
<ClCompile Include="ArpHelper.cpp" />
200201
<ClCompile Include="Certificates.cpp" />
201202
<ClCompile Include="CheckpointDatabase.cpp" />
202203
<ClCompile Include="Command.cpp" />

‎src/AppInstallerCLITests/AppInstallerCLITests.vcxproj.filters‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -326,6 +326,9 @@
326326
<ClCompile Include="DateTime.cpp">
327327
<Filter>Source Files\Common</Filter>
328328
</ClCompile>
329+
<ClCompile Include="ArpHelper.cpp">
330+
<Filter>Source Files\Repository</Filter>
331+
</ClCompile>
329332
</ItemGroup>
330333
<ItemGroup>
331334
<None Include="PropertySheet.props" />

‎src/AppInstallerCLITests/ArpHelper.cpp‎

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
// Copyright (c) Microsoft Corporation.
2+
// Licensed under the MIT License.
3+
#include "pch.h"
4+
#include "TestCommon.h"
5+
#include "TestHooks.h"
6+
#include "Microsoft/ARPHelper.h"
7+
#include "AppInstallerStrings.h"
8+
9+
using namespace AppInstaller::Manifest;
10+
using namespace AppInstaller::Repository::Microsoft;
11+
using namespace AppInstaller::Utility;
12+
using namespace AppInstaller::Registry;
13+
14+
15+
TEST_CASE("ARPHelper_Watcher", "[ARPHelper]")
16+
{
17+
ARPHelper helper;
18+
19+
wil::unique_event callbackEvent;
20+
callbackEvent.create();
21+
22+
ScopeEnum scopeCallback = ScopeEnum::Unknown;
23+
Architecture architectureCallback = Architecture::Unknown;
24+
25+
ScopeEnum scopeTarget = ScopeEnum::Machine;
26+
Architecture architectureTarget = Architecture::X86;
27+
28+
auto fakeRoot = TestCommon::RegCreateVolatileTestRoot();
29+
TestHook::SetGetARPKey_Override arpOverride([&](ScopeEnum scope, Architecture arch)
30+
{
31+
if (scope == scopeTarget && arch == architectureTarget)
32+
{
33+
return Key(fakeRoot.get(), L"");
34+
}
35+
else
36+
{
37+
return Key{};
38+
}
39+
});
40+
41+
auto watchers = helper.CreateRegistryWatchers(scopeTarget, [&](ScopeEnum scope, Architecture arch, wil::RegistryChangeKind)
42+
{
43+
scopeCallback = scope;
44+
architectureCallback = arch;
45+
callbackEvent.SetEvent();
46+
});
47+
48+
auto arpKey = helper.GetARPKey(scopeTarget, architectureTarget);
49+
REQUIRE(!!arpKey);
50+
51+
GUID guid;
52+
std::ignore = CoCreateGuid(&guid);
53+
std::ostringstream stream;
54+
stream << guid;
55+
56+
auto testKey = TestCommon::RegCreateVolatileSubKey(arpKey, ConvertToUTF16(stream.str()));
57+
58+
REQUIRE(callbackEvent.wait(1000));
59+
REQUIRE(scopeTarget == scopeCallback);
60+
REQUIRE(architectureTarget == architectureCallback);
61+
62+
// Reset for changing a value
63+
scopeCallback = ScopeEnum::Unknown;
64+
architectureCallback = Architecture::Unknown;
65+
66+
TestCommon::SetRegistryValue(testKey.get(), L"testValue", L"valueValue");
67+
68+
REQUIRE(callbackEvent.wait(1000));
69+
REQUIRE(scopeTarget == scopeCallback);
70+
REQUIRE(architectureTarget == architectureCallback);
71+
}

0 commit comments

Comments
 (0)