Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve performance of XmlDependencies IsDependenciesFile #602

Open
wants to merge 10 commits into
base: master
Choose a base branch
from
2 changes: 2 additions & 0 deletions build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -2246,6 +2246,8 @@ task gitTagRelease(type: Exec) {
// - Conflicting dependencies
// - FAT ABI vs. single ABI selection

// TODO: Build and run NUnit tests for #602

// TODO: iOS Resolver tests (on OSX only)
// - Import plugin test Cocoapods bootstrap
// - Pod specification, with reflection & XML, validate both are present in the
Expand Down
10 changes: 6 additions & 4 deletions source/AndroidResolver/src/PlayServicesResolver.cs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ namespace GooglePlayServices {
using System;
using System.Collections.Generic;
using System.IO;
using System.Linq;
using System.Text.RegularExpressions;
using System.Threading;
using System.Xml;
Expand Down Expand Up @@ -992,7 +993,7 @@ private static bool Initialize() {
}

// Monitor Android dependency XML files to perform auto-resolution.
AddAutoResolutionFilePatterns(xmlDependencies.fileRegularExpressions);
autoResolveFilePatterns.Add(XmlDependencies.IsDependenciesFile);

svcSupport = PlayServicesSupport.CreateInstance(
"PlayServicesResolver",
Expand Down Expand Up @@ -1079,15 +1080,16 @@ internal static void Log(string message, Google.LogLevel level = LogLevel.Info)
/// <summary>
/// Patterns of files that are monitored to trigger auto resolution.
/// </summary>
private static HashSet<Regex> autoResolveFilePatterns = new HashSet<Regex>();
private static HashSet<Func<string, bool>> autoResolveFilePatterns = new HashSet<Func<string, bool>>();

/// <summary>
/// Add file patterns to monitor to trigger auto resolution.
/// </summary>
/// <param name="patterns">Set of file patterns to monitor to trigger auto
/// resolution.</param>
public static void AddAutoResolutionFilePatterns(IEnumerable<Regex> patterns) {
autoResolveFilePatterns.UnionWith(patterns);
// Only regex patterns are supported in the public API, but a more performant default is used internally.
autoResolveFilePatterns.UnionWith(patterns.Select<Regex, Func<string, bool>>(p => p.IsMatch));
}

/// <summary>
Expand All @@ -1099,7 +1101,7 @@ private static bool CheckFilesForAutoResolution(HashSet<string> filesToCheck) {
bool resolve = false;
foreach (var asset in filesToCheck) {
foreach (var pattern in autoResolveFilePatterns) {
if (pattern.Match(asset).Success) {
if (pattern.Invoke(asset)) {
Log(String.Format("Found asset {0} matching {1}, attempting " +
"auto-resolution.",
asset, pattern.ToString()),
Expand Down
20 changes: 5 additions & 15 deletions source/AndroidResolver/src/XmlDependencies.cs
Original file line number Diff line number Diff line change
Expand Up @@ -28,14 +28,6 @@ namespace GooglePlayServices {
/// </summary>
internal class XmlDependencies {

/// <summary>
/// Set of regular expressions that match files which contain dependency
/// specifications.
/// </summary>
internal HashSet<Regex> fileRegularExpressions = new HashSet<Regex> {
new Regex(@".*[/\\]Editor[/\\].*Dependencies\.xml$")
};

/// <summary>
/// Human readable name for dependency files managed by this class.
/// </summary>
Expand All @@ -44,15 +36,13 @@ internal class XmlDependencies {
/// <summary>
/// Determines whether a filename matches an XML dependencies file.
/// </summary>
/// <param name="filename"></param>
/// <returns>true if it is a match, false otherwise.</returns>
internal bool IsDependenciesFile(string filename) {
foreach (var regex in fileRegularExpressions) {
if (regex.Match(filename).Success) {
return true;
}
internal static bool IsDependenciesFile(string filename) {
if (!filename.EndsWith("Dependencies.xml")) {
return false;
}
return false;
// This method was optimized to avoid using regex after profiling results from a large Unity project.
return filename.Contains("/Editor/") || filename.Contains(@"\Editor\");
}

/// <summary>
Expand Down
61 changes: 61 additions & 0 deletions source/AndroidResolver/unit_tests/AndroidResolverTests.csproj
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
<?xml version="1.0" encoding="utf-8"?>
<Project ToolsVersion="4.0" DefaultTargets="Build" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
<Import Project="$(MSBuildExtensionsPath)\$(MSBuildToolsVersion)\Microsoft.Common.props" Condition="Exists('$(MSBuildExtensionsPath)\$(MSBuildToolsVersion)\Microsoft.Common.props')" />
<PropertyGroup>
<Configuration Condition=" '$(Configuration)' == '' ">Debug</Configuration>
<Platform Condition=" '$(Platform)' == '' ">AnyCPU</Platform>
<ProjectGuid>{12F40968-4B80-4DCF-8D51-4C8E06F9CC4B}</ProjectGuid>
<ProjectTypeGuids>{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}</ProjectTypeGuids>
<OutputType>Library</OutputType>
<AppDesignerFolder>Properties</AppDesignerFolder>
<RootNamespace>Google</RootNamespace>
<AssemblyName>Google.AndroidResolverTests</AssemblyName>
<TargetFrameworkVersion>v3.5</TargetFrameworkVersion>
<FileAlignment>512</FileAlignment>
</PropertyGroup>
<PropertyGroup Condition=" '$(Configuration)|$(Platform)' == 'Debug|AnyCPU' ">
<PlatformTarget>AnyCPU</PlatformTarget>
<DebugSymbols>true</DebugSymbols>
<DebugType>full</DebugType>
<Optimize>false</Optimize>
<OutputPath>bin\Debug\</OutputPath>
<DefineConstants>DEBUG;TRACE</DefineConstants>
<ErrorReport>prompt</ErrorReport>
<WarningLevel>4</WarningLevel>
</PropertyGroup>
<PropertyGroup Condition=" '$(Configuration)|$(Platform)' == 'Release|AnyCPU' ">
<PlatformTarget>AnyCPU</PlatformTarget>
<DebugType>pdbonly</DebugType>
<Optimize>true</Optimize>
<OutputPath>bin\Release\</OutputPath>
<DefineConstants>TRACE</DefineConstants>
<ErrorReport>prompt</ErrorReport>
<WarningLevel>4</WarningLevel>
</PropertyGroup>
<ItemGroup>
<Reference Include="System" />
<Reference Include="System.Core" />
<Reference Include="System.Data" />
<Reference Include="System.Xml" />
<Reference Include="nunit.framework, Version=3.5.0.0, Culture=neutral, PublicKeyToken=2638cd05610744eb" />
</ItemGroup>
<ItemGroup>
<Compile Include="XmlDependenciesTests.cs" />
<Compile Include="Properties\AssemblyInfo.cs" />
</ItemGroup>
<ItemGroup>
<ProjectReference Include="..\AndroidResolver.csproj">
<Project>{82eedfbe-afe4-4def-99d9-bc929747dd9a}</Project>
<Name>AndroidResolver</Name>
</ProjectReference>
</ItemGroup>
<Import Project="$(MSBuildToolsPath)\Microsoft.CSharp.targets" />
<!-- To modify your build process, add your task inside one of the targets below and uncomment it.
Other similar extension points exist, see Microsoft.Common.targets.
<Target Name="BeforeBuild">
</Target>
<Target Name="AfterBuild">
</Target>
-->

</Project>
35 changes: 35 additions & 0 deletions source/AndroidResolver/unit_tests/Properties/AssemblyInfo.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
using System.Reflection;
using System.Runtime.InteropServices;

// General Information about an assembly is controlled through the following
// set of attributes. Change these attribute values to modify the information
// associated with an assembly.
[assembly: AssemblyTitle("AndroidResolverTests")]
[assembly: AssemblyDescription("")]
[assembly: AssemblyConfiguration("")]
[assembly: AssemblyCompany("")]
[assembly: AssemblyProduct("AndroidResolverTests")]
[assembly: AssemblyCopyright("Copyright © 2023")]
[assembly: AssemblyTrademark("")]
[assembly: AssemblyCulture("")]

// Setting ComVisible to false makes the types in this assembly not visible
// to COM components. If you need to access a type in this assembly from
// COM, set the ComVisible attribute to true on that type.
[assembly: ComVisible(false)]

// The following GUID is for the ID of the typelib if this project is exposed to COM
[assembly: Guid("12F40968-4B80-4DCF-8D51-4C8E06F9CC4B")]

// Version information for an assembly consists of the following four values:
//
// Major Version
// Minor Version
// Build Number
// Revision
//
// You can specify all the values or you can default the Build and Revision Numbers
// by using the '*' as shown below:
// [assembly: AssemblyVersion("1.0.*")]
[assembly: AssemblyVersion("1.0.0.0")]
[assembly: AssemblyFileVersion("1.0.0.0")]
28 changes: 28 additions & 0 deletions source/AndroidResolver/unit_tests/XmlDependenciesTests.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
using System.Text.RegularExpressions;

namespace GooglePlayServices.Tests {
using System;
using NUnit.Framework;

[TestFixture]
public class XmlDependenciesTests
{
[TestCase("Assets/Editor/Dependencies.xml")]
[TestCase("Assets/Editor/MyFolder/Dependencies.xml")]
[TestCase("Editor/Dependencies.xml")]
[TestCase("Editor/MyFolder/Dependencies.xml")]
[TestCase("Assets/Editor/SomeDependencies.xml")]
[TestCase("Assets/MyEditorCode/Dependencies.xml")]
[TestCase("Assets/MyEditorCode/SomeDependencies.xml")]
[TestCase("Assets/Editor/")]
[TestCase("Assets/Editor/Dependendencies")]
public void IsDependenciesFileReturnsExpected(string path) {
bool actualResult = XmlDependencies.IsDependenciesFile(path);

// This was the previous implementation before the optimization attempt and acts as a test reference.
bool expectedResult = Regex.IsMatch(input: path, pattern: @".*[/\\]Editor[/\\].*Dependencies\.xml$");

Assert.AreEqual(expectedResult, actualResult);
}
}
}
4 changes: 4 additions & 0 deletions source/AndroidResolver/unit_tests/packages.config
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
<?xml version="1.0" encoding="utf-8"?>
<packages>
<package id="NUnit" version="2.6.3" targetFramework="net45" />
</packages>
6 changes: 6 additions & 0 deletions source/ExternalDependencyManager.sln
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "PackageManagerClientIntegra
EndProject
Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "PackageMigratorIntegrationTests", "PackageManagerResolver\test\PackageMigratorIntegrationTests.csproj", "{4DBDEE33-4B6C-A866-93FE-04C15486BB03}"
EndProject
Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "AndroidResolverTests", "AndroidResolver\unit_tests\AndroidResolverTests.csproj", "{12F40968-4B80-4DCF-8D51-4C8E06F9CC4B}"
EndProject
Global
GlobalSection(SolutionConfigurationPlatforms) = preSolution
Debug|Any CPU = Debug|Any CPU
Expand Down Expand Up @@ -85,5 +87,9 @@ Global
{4DBDEE33-4B6C-A866-93FE-04C15486BB03}.Debug|Any CPU.Build.0 = Debug|Any CPU
{4DBDEE33-4B6C-A866-93FE-04C15486BB03}.Release|Any CPU.ActiveCfg = Release|Any CPU
{4DBDEE33-4B6C-A866-93FE-04C15486BB03}.Release|Any CPU.Build.0 = Release|Any CPU
{12F40968-4B80-4DCF-8D51-4C8E06F9CC4B}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
{12F40968-4B80-4DCF-8D51-4C8E06F9CC4B}.Debug|Any CPU.Build.0 = Debug|Any CPU
{12F40968-4B80-4DCF-8D51-4C8E06F9CC4B}.Release|Any CPU.ActiveCfg = Release|Any CPU
{12F40968-4B80-4DCF-8D51-4C8E06F9CC4B}.Release|Any CPU.Build.0 = Release|Any CPU
EndGlobalSection
EndGlobal
2 changes: 1 addition & 1 deletion source/IOSResolver/src/IOSResolver.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1939,7 +1939,7 @@ private static void OnPostprocessAllAssets(string[] importedAssets,
var changedAssets = new List<string>(importedAssets);
changedAssets.AddRange(deletedAssets);
foreach (var asset in changedAssets) {
dependencyFileChanged = xmlDependencies.IsDependenciesFile(asset);
dependencyFileChanged = XmlDependencies.IsDependenciesFile(asset);
if (dependencyFileChanged) break;
}
if (dependencyFileChanged) RefreshXmlDependencies();
Expand Down