Performance improvements (#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.

.github/actions/spelling/expect.txt

@@ -131,6 +131,7 @@ ENDSESSION
 EPester
 epth
 EQU
+errcode
 errmsg
 ERRORONEXIT
 ESource

azure-pipelines.yml

@@ -123,9 +123,11 @@ jobs:
                     /p:UapAppxPackageBuildMode=SideloadOnly'
 
   - task: CopyFiles@2
-    displayName: 'Copy WindowsPackageManager.dll Symbols to artifacts folder'
+    displayName: 'Copy Symbols to artifacts folder'
     inputs:
-      Contents: '$(buildOutDir)\WindowsPackageManager\WindowsPackageManager.pdb'
+      Contents: |
+          $(buildOutDir)\WindowsPackageManager\WindowsPackageManager.pdb
+          $(buildOutDir)\Microsoft.Management.Configuration\Microsoft.Management.Configuration.pdb
       TargetFolder: '$(artifactsDir)'
     condition: succeededOrFailed()
 
@@ -306,7 +308,7 @@ jobs:
       platform: '$(buildPlatform)'
       configuration: '$(BuildConfiguration)'
       diagnosticsEnabled: true
-    condition: succeededOrFailed()
+    condition: and(succeededOrFailed(), eq(variables.buildPlatform, 'x86'))
 
   - task: PowerShell@2
     displayName: Prepare for Microsoft.Management.Configuration.UnitTests (OutOfProc)

src/AppInstallerCLICore/Workflows/InstallFlow.cpp

@@ -16,6 +16,7 @@
 #include "WorkflowBase.h"
 #include "DependenciesFlow.h"
 #include "PromptFlow.h"
+#include "SourceFlow.h"
 #include <AppInstallerMsixInfo.h>
 #include <AppInstallerDeployment.h>
 #include <AppInstallerSynchronization.h>
@@ -553,6 +554,7 @@ namespace AppInstaller::CLI::Workflow
             Workflow::ReportExecutionStage(ExecutionStage::PostExecution) <<
             Workflow::ReportARPChanges <<
             Workflow::RecordInstall <<
+            Workflow::ForceInstalledCacheUpdate <<
             Workflow::RemoveInstaller <<
             Workflow::DisplayInstallationNotes;
     }

src/AppInstallerCLICore/Workflows/SourceFlow.cpp

@@ -301,4 +301,10 @@ namespace AppInstaller::CLI::Workflow
             }
         }
     }
+
+    void ForceInstalledCacheUpdate(Execution::Context&)
+    {
+        // Creating this object is currently sufficient to mark the cache as needing an update for the next time it is opened.
+        Repository::Source ignore{ Repository::PredefinedSource::InstalledForceCacheUpdate };
+    }
 }

src/AppInstallerCLICore/Workflows/SourceFlow.h

@@ -77,4 +77,10 @@ namespace AppInstaller::CLI::Workflow
     // Inputs: SourceList
     // Outputs: None
     void ExportSourceList(Execution::Context& context);
+
+    // Forces an update to the cache of installed packages.
+    // Required Args: None
+    // Inputs: None
+    // Outputs: None
+    void ForceInstalledCacheUpdate(Execution::Context& context);
 }

src/AppInstallerCLIE2ETests/Constants.cs

@@ -82,6 +82,7 @@ public class Constants
         public const string PortableExePackageId = "AppInstallerTest.TestPortableExe";
         public const string PortableExeWithCommandPackageId = "AppInstallerTest.TestPortableExeWithCommand";
 
+        public const string ExeInstalledDefaultProductCode = "{A499DD5E-8DC5-4AD2-911A-BCD0263295E9}";
         public const string MsiInstallerProductCode = "{A5D36CF1-1993-4F63-BFB4-3ACD910D36A1}";
         public const string MsixInstallerName = "6c6338fe-41b7-46ca-8ba6-b5ad5312bb0e";
         public const string MsixInstallerPackageFamilyName = "6c6338fe-41b7-46ca-8ba6-b5ad5312bb0e_8wekyb3d8bbwe";

src/AppInstallerCLIE2ETests/Interop/InstallInterop.cs

@@ -657,5 +657,72 @@ public void GetApplicableInstaller()
             Assert.AreEqual(PackageInstallerScope.User, packageInstallerInfo.Scope);
             Assert.AreEqual("en-US", packageInstallerInfo.Locale);
         }
+
+        /// <summary>
+        /// Install exe and verify that we can find it as installed after.
+        /// </summary>
+        /// <returns>A <see cref="Task"/> representing the asynchronous unit test.</returns>
+        [Test]
+        public async Task InstallExe_VerifyInstalledCatalog()
+        {
+            var installedCatalogReference = this.packageManager.GetLocalPackageCatalog(LocalPackageCatalog.InstalledPackages);
+
+            // Ensure package is not installed
+            var installedResult = this.FindAllPackages(installedCatalogReference, PackageMatchField.ProductCode, PackageFieldMatchOption.Equals, Constants.ExeInstalledDefaultProductCode);
+            Assert.IsNotNull(installedResult);
+            Assert.AreEqual(0, installedResult.Count);
+
+            // Find package
+            var searchResult = this.FindOnePackage(this.testSource, PackageMatchField.Id, PackageFieldMatchOption.Equals, "AppInstallerTest.TestExeInstaller");
+
+            // Configure installation
+            var installOptions = this.TestFactory.CreateInstallOptions();
+            installOptions.PackageInstallMode = PackageInstallMode.Silent;
+            installOptions.PreferredInstallLocation = this.installDir;
+            installOptions.AcceptPackageAgreements = true;
+
+            // Install
+            var installResult = await this.packageManager.InstallPackageAsync(searchResult.CatalogPackage, installOptions);
+
+            // Assert
+            Assert.AreEqual(InstallResultStatus.Ok, installResult.Status);
+
+            // Check installed catalog after
+            this.FindOnePackage(installedCatalogReference, PackageMatchField.ProductCode, PackageFieldMatchOption.Equals, Constants.ExeInstalledDefaultProductCode);
+
+            Assert.True(TestCommon.VerifyTestExeInstalledAndCleanup(this.installDir));
+        }
+
+        /// <summary>
+        /// Install msix and verify that we can find it as installed after.
+        /// </summary>
+        /// <returns>A <see cref="Task"/> representing the asynchronous unit test.</returns>
+        [Test]
+        public async Task InstallMSIX_VerifyInstalledCatalog()
+        {
+            var installedCatalogReference = this.packageManager.GetLocalPackageCatalog(LocalPackageCatalog.InstalledPackages);
+
+            // Ensure package is not installed
+            var installedResult = this.FindAllPackages(installedCatalogReference, PackageMatchField.PackageFamilyName, PackageFieldMatchOption.Equals, Constants.MsixInstallerPackageFamilyName);
+            Assert.IsNotNull(installedResult);
+            Assert.AreEqual(0, installedResult.Count);
+
+            // Find package
+            var searchResult = this.FindOnePackage(this.testSource, PackageMatchField.Name, PackageFieldMatchOption.Equals, "TestMsixInstaller");
+
+            // Configure installation
+            var installOptions = this.TestFactory.CreateInstallOptions();
+
+            // Install
+            var installResult = await this.packageManager.InstallPackageAsync(searchResult.CatalogPackage, installOptions);
+
+            // Assert
+            Assert.AreEqual(InstallResultStatus.Ok, installResult.Status);
+
+            // Check installed catalog after
+            this.FindOnePackage(installedCatalogReference, PackageMatchField.PackageFamilyName, PackageFieldMatchOption.Equals, Constants.MsixInstallerPackageFamilyName);
+
+            Assert.True(TestCommon.VerifyTestMsixInstalledAndCleanup());
+        }
     }
 }

src/AppInstallerCLITests/AppInstallerCLITests.vcxproj

@@ -197,6 +197,7 @@
     <ClCompile Include="Archive.cpp" />
     <ClCompile Include="Argument.cpp" />
     <ClCompile Include="ARPChanges.cpp" />
+    <ClCompile Include="ArpHelper.cpp" />
     <ClCompile Include="Certificates.cpp" />
     <ClCompile Include="CheckpointDatabase.cpp" />
     <ClCompile Include="Command.cpp" />

src/AppInstallerCLITests/AppInstallerCLITests.vcxproj.filters

@@ -326,6 +326,9 @@
     <ClCompile Include="DateTime.cpp">
       <Filter>Source Files\Common</Filter>
     </ClCompile>
+    <ClCompile Include="ArpHelper.cpp">
+      <Filter>Source Files\Repository</Filter>
+    </ClCompile>
   </ItemGroup>
   <ItemGroup>
     <None Include="PropertySheet.props" />

src/AppInstallerCLITests/ArpHelper.cpp

@@ -0,0 +1,71 @@
+// Copyright (c) Microsoft Corporation.
+// Licensed under the MIT License.
+#include "pch.h"
+#include "TestCommon.h"
+#include "TestHooks.h"
+#include "Microsoft/ARPHelper.h"
+#include "AppInstallerStrings.h"
+
+using namespace AppInstaller::Manifest;
+using namespace AppInstaller::Repository::Microsoft;
+using namespace AppInstaller::Utility;
+using namespace AppInstaller::Registry;
+
+
+TEST_CASE("ARPHelper_Watcher", "[ARPHelper]")
+{
+    ARPHelper helper;
+
+    wil::unique_event callbackEvent;
+    callbackEvent.create();
+
+    ScopeEnum scopeCallback = ScopeEnum::Unknown;
+    Architecture architectureCallback = Architecture::Unknown;
+
+    ScopeEnum scopeTarget = ScopeEnum::Machine;
+    Architecture architectureTarget = Architecture::X86;
+
+    auto fakeRoot = TestCommon::RegCreateVolatileTestRoot();
+    TestHook::SetGetARPKey_Override arpOverride([&](ScopeEnum scope, Architecture arch)
+        {
+            if (scope == scopeTarget && arch == architectureTarget)
+            {
+                return Key(fakeRoot.get(), L"");
+            }
+            else
+            {
+                return Key{};
+            }
+        });
+
+    auto watchers = helper.CreateRegistryWatchers(scopeTarget, [&](ScopeEnum scope, Architecture arch, wil::RegistryChangeKind)
+        {
+            scopeCallback = scope;
+            architectureCallback = arch;
+            callbackEvent.SetEvent();
+        });
+
+    auto arpKey = helper.GetARPKey(scopeTarget, architectureTarget);
+    REQUIRE(!!arpKey);
+
+    GUID guid;
+    std::ignore = CoCreateGuid(&guid);
+    std::ostringstream stream;
+    stream << guid;
+
+    auto testKey = TestCommon::RegCreateVolatileSubKey(arpKey, ConvertToUTF16(stream.str()));
+
+    REQUIRE(callbackEvent.wait(1000));
+    REQUIRE(scopeTarget == scopeCallback);
+    REQUIRE(architectureTarget == architectureCallback);
+
+    // Reset for changing a value
+    scopeCallback = ScopeEnum::Unknown;
+    architectureCallback = Architecture::Unknown;
+
+    TestCommon::SetRegistryValue(testKey.get(), L"testValue", L"valueValue");
+
+    REQUIRE(callbackEvent.wait(1000));
+    REQUIRE(scopeTarget == scopeCallback);
+    REQUIRE(architectureTarget == architectureCallback);
+}

src/AppInstallerCLITests/PredefinedInstalledSource.cpp

@@ -7,6 +7,7 @@
 #include <AppInstallerStrings.h>
 #include <Microsoft/PredefinedInstalledSourceFactory.h>
 #include <Microsoft/ARPHelper.h>
+#include <Microsoft/SQLiteIndexSource.h>
 
 using namespace std::string_literals;
 using namespace std::string_view_literals;
@@ -18,6 +19,7 @@ using namespace AppInstaller::Runtime;
 using namespace AppInstaller::Utility;
 
 using SQLiteIndex = AppInstaller::Repository::Microsoft::SQLiteIndex;
+using SQLiteIndexSource = AppInstaller::Repository::Microsoft::SQLiteIndexSource;
 using Factory = AppInstaller::Repository::Microsoft::PredefinedInstalledSourceFactory;
 using ARPHelper = AppInstaller::Repository::Microsoft::ARPHelper;
 
@@ -400,3 +402,82 @@ TEST_CASE("PredefinedInstalledSource_Search", "[installed][list]")
 
     REQUIRE_FALSE(results.Matches.empty());
 }
+
+std::string GetDatabaseIdentifier(const std::shared_ptr<Repository::ISource>& source)
+{
+    return reinterpret_cast<SQLiteIndexSource*>(source->CastTo(ISourceType::SQLiteIndexSource))->GetIndex().GetDatabaseIdentifier();
+}
+
+void RequirePackagesHaveSameNames(std::shared_ptr<ISource>& source1, std::shared_ptr<ISource>& source2)
+{
+    auto result1 = source1->Search({});
+    REQUIRE(!result1.Matches.empty());
+
+    // Ensure that all packages have the same name values
+    for (const auto& match : result1.Matches)
+    {
+        std::string packageId = match.Package->GetProperty(PackageProperty::Id).get();
+        INFO(packageId);
+
+        SearchRequest id2;
+        id2.Inclusions.emplace_back(PackageMatchFilter{ PackageMatchField::Id, MatchType::CaseInsensitive, packageId });
+        auto result2 = source2->Search(id2);
+        REQUIRE(result2.Matches.size() == 1);
+        REQUIRE(match.Package->GetProperty(PackageProperty::Name) == result2.Matches[0].Package->GetProperty(PackageProperty::Name));
+    }
+}
+
+TEST_CASE("PredefinedInstalledSource_Create_Cached", "[installed][list][installed-cache]")
+{
+    auto source1 = CreatePredefinedInstalledSource();
+    auto source2 = CreatePredefinedInstalledSource();
+
+    // Ensure the same identifier (which should mean the cache was not updated)
+    REQUIRE(
+        GetDatabaseIdentifier(source1)
+        ==
+        GetDatabaseIdentifier(source2)
+    );
+
+    RequirePackagesHaveSameNames(source1, source2);
+    RequirePackagesHaveSameNames(source2, source1);
+}
+
+TEST_CASE("PredefinedInstalledSource_Create_ForceCacheUpdate", "[installed][list][installed-cache]")
+{
+    auto source1 = CreatePredefinedInstalledSource();
+    auto source2 = CreatePredefinedInstalledSource(Factory::Filter::NoneWithForcedCacheUpdate);
+
+    // Ensure different identifier (which should mean the cache was updated)
+    REQUIRE(
+        GetDatabaseIdentifier(source1)
+        !=
+        GetDatabaseIdentifier(source2)
+    );
+
+    RequirePackagesHaveSameNames(source1, source2);
+    RequirePackagesHaveSameNames(source2, source1);
+}
+
+TEST_CASE("PredefinedInstalledSource_Create_ForceCacheUpdate_StillCached", "[installed][list][installed-cache]")
+{
+    auto source1 = CreatePredefinedInstalledSource();
+    auto source2 = CreatePredefinedInstalledSource(Factory::Filter::NoneWithForcedCacheUpdate);
+    auto source3 = CreatePredefinedInstalledSource();
+
+    CAPTURE(GetDatabaseIdentifier(source1), GetDatabaseIdentifier(source2), GetDatabaseIdentifier(source3));
+
+    // Ensure different identifier (which should mean the cache was updated)
+    REQUIRE(
+        GetDatabaseIdentifier(source1)
+        !=
+        GetDatabaseIdentifier(source2)
+    );
+
+    // Ensure the same identifier (which should mean the cache was not updated)
+    REQUIRE(
+        GetDatabaseIdentifier(source2)
+        ==
+        GetDatabaseIdentifier(source3)
+    );
+}