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

[Xamarin.Android.Build.Tasks] do not use %(HintPath) #3068

Merged
merged 3 commits into from
May 8, 2019

Conversation

jonathanpeppers
Copy link
Member

We noticed some odd behavior from MSBuild such as:

ReferenceDependencyPaths
    C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\Common7\IDE\ReferenceAssemblies\Microsoft\Framework\MonoAndroid\v1.0\Facades\netstandard.dll
        HintPath = ..\..\..\packages\Newtonsoft.Json.11.0.2\lib\netstandard2.0\Newtonsoft.Json.dll
    C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\Common7\IDE\ReferenceAssemblies\Microsoft\Framework\MonoAndroid\v1.0\System.Runtime.Serialization.dll
        HintPath = ..\..\..\packages\Xamarin.Forms.3.1.0.583944\lib\MonoAndroid10\Xamarin.Forms.Platform.Android.dll

%(HintPath) is leftover metadata from the @(ReferencePath) item
group, so it points to the original assembly???

We currently use %(HintPath) in two places, but it seems like we
don't need to use it at all? @(ReferencePath) and
@(ReferenceDependencyPaths) will both have the full path to all of
the assemblies in ITaskItem.ItemSpec.

I was able to simplify two of our tasks and just use ItemSpec:

  • <ResolveLibraryProjectImports/>
  • <GenerateResourceDesigner/>

I also did some general refactoring in <GenerateResourceDesigner/>:

  • Reorganized the top-level foreach loop, to more closely match what
    we were already doing in <ResolveLibraryProjectImports/>.
  • Cleaned up some general LINQ usage.
  • A List<string> of assembly names could just be a
    List<AssemblyDefinition>, to avoid some LINQ.
  • Fixed spelling/used string interpolation in log messages.

Results

I think I'm seeing a ~50ms improvement in
<GenerateResourceDesigner/> for an initial build:

Before:
205 ms  GenerateResourceDesigner                   1 calls
After:
152 ms  GenerateResourceDesigner                   1 calls

Future Work

  • It might be worth porting <GenerateResourceDesigner/> to use
    System.Reflection.Metadata instead of Mono.Cecil at some point.

We noticed some odd behavior from MSBuild such as:

    ReferenceDependencyPaths
        C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\Common7\IDE\ReferenceAssemblies\Microsoft\Framework\MonoAndroid\v1.0\Facades\netstandard.dll
            HintPath = ..\..\..\packages\Newtonsoft.Json.11.0.2\lib\netstandard2.0\Newtonsoft.Json.dll
        C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\Common7\IDE\ReferenceAssemblies\Microsoft\Framework\MonoAndroid\v1.0\System.Runtime.Serialization.dll
            HintPath = ..\..\..\packages\Xamarin.Forms.3.1.0.583944\lib\MonoAndroid10\Xamarin.Forms.Platform.Android.dll

`%(HintPath)` is leftover metadata from the `@(ReferencePath)` item
group, so it points to the original assembly???

We currently use `%(HintPath)` in two places, but it seems like we
don't need to use it at all? `@(ReferencePath)` and
`@(ReferenceDependencyPaths)` will both have the full path to all of
the assemblies in `ITaskItem.ItemSpec`.

I was able to simplify two of our tasks and just use `ItemSpec`:

* `<ResolveLibraryProjectImports/>`
* `<GenerateResourceDesigner/>`

I also did some general refactoring in `<GenerateResourceDesigner/>`:

* Reorganized the top-level `foreach` loop, to more closely match what
  we were already doing in `<ResolveLibraryProjectImports/>`.
* Cleaned up some general LINQ usage.
* A `List<string>` of assembly names could just be a
  `List<AssemblyDefinition>`, to avoid some LINQ.
* Fixed spelling/used string interpolation in log messages.

~~ Results ~~

I think I'm seeing a ~50ms improvement in
`<GenerateResourceDesigner/>` for an initial build:

    Before:
    205 ms  GenerateResourceDesigner                   1 calls
    After:
    152 ms  GenerateResourceDesigner                   1 calls

~~ Future Work ~~

* It might be worth porting `<GenerateResourceDesigner/>` to use
  System.Reflection.Metadata instead of Mono.Cecil at some point.
@jonathanpeppers
Copy link
Member Author

FYI I asked about in #msbuild on Slack, the thought is:

  • @(ReferenceDependencyPaths) is just inheriting all metadata from the original @(ReferencePath) item group.
  • They are afraid to change %(HintPath) on @(ReferenceDependencyPaths) because someone might be relying on it now...

@jonpryor jonpryor merged commit 0ad06fc into dotnet:master May 8, 2019
@github-actions github-actions bot locked and limited conversation to collaborators Jan 30, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants