Skip to content

Commit

Permalink
Performance improvements (#3867)
Browse files Browse the repository at this point in the history
Cherry-pick of #3808 , then fixing up the changes (mostly namespace
changes, but also bring one extra function over) for the fact that the
SQLite base code moved around.
  • Loading branch information
JohnMcPMS committed Nov 9, 2023
1 parent 5267f91 commit cddb7f8
Show file tree
Hide file tree
Showing 44 changed files with 1,041 additions and 227 deletions.
1 change: 1 addition & 0 deletions .github/actions/spelling/expect.txt
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,7 @@ ENDSESSION
EPester
epth
EQU
errcode
errmsg
ERRORONEXIT
ESource
Expand Down
10 changes: 7 additions & 3 deletions azure-pipelines.yml
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ pr:
branches:
include:
- master
- release-v1.6
paths:
include:
- azure-pipelines.yml
Expand Down Expand Up @@ -123,9 +124,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()

Expand Down Expand Up @@ -305,7 +308,8 @@ jobs:
codeCoverageEnabled: true
platform: '$(buildPlatform)'
configuration: '$(BuildConfiguration)'
condition: succeededOrFailed()
diagnosticsEnabled: true
condition: and(succeededOrFailed(), eq(variables.buildPlatform, 'x86'))

- task: PowerShell@2
displayName: Prepare for Microsoft.Management.Configuration.UnitTests (OutOfProc)
Expand Down
2 changes: 2 additions & 0 deletions src/AppInstallerCLICore/Workflows/InstallFlow.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
#include "WorkflowBase.h"
#include "DependenciesFlow.h"
#include "PromptFlow.h"
#include "SourceFlow.h"
#include <AppInstallerMsixInfo.h>
#include <AppInstallerDeployment.h>
#include <AppInstallerSynchronization.h>
Expand Down Expand Up @@ -531,6 +532,7 @@ namespace AppInstaller::CLI::Workflow
Workflow::ReportExecutionStage(ExecutionStage::PostExecution) <<
Workflow::ReportARPChanges <<
Workflow::RecordInstall <<
Workflow::ForceInstalledCacheUpdate <<
Workflow::RemoveInstaller <<
Workflow::DisplayInstallationNotes;
}
Expand Down
6 changes: 6 additions & 0 deletions src/AppInstallerCLICore/Workflows/SourceFlow.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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 };
}
}
6 changes: 6 additions & 0 deletions src/AppInstallerCLICore/Workflows/SourceFlow.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
1 change: 1 addition & 0 deletions src/AppInstallerCLIE2ETests/Constants.cs
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,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";
Expand Down
67 changes: 67 additions & 0 deletions src/AppInstallerCLIE2ETests/Interop/InstallInterop.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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());
}
}
}
1 change: 1 addition & 0 deletions src/AppInstallerCLITests/AppInstallerCLITests.vcxproj
Original file line number Diff line number Diff line change
Expand Up @@ -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="Command.cpp" />
<ClCompile Include="Completion.cpp" />
Expand Down
3 changes: 3 additions & 0 deletions src/AppInstallerCLITests/AppInstallerCLITests.vcxproj.filters
Original file line number Diff line number Diff line change
Expand Up @@ -317,6 +317,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" />
Expand Down
71 changes: 71 additions & 0 deletions src/AppInstallerCLITests/ArpHelper.cpp
Original file line number Diff line number Diff line change
@@ -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);
}
81 changes: 81 additions & 0 deletions src/AppInstallerCLITests/PredefinedInstalledSource.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;

Expand Down Expand Up @@ -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)
);
}
Loading

0 comments on commit cddb7f8

Please sign in to comment.