Skip to content

Use stripped-down resolver for perf (no GAC, deferred reading mode)#54

Merged
sharwell merged 3 commits into
tunnelvisionlabs:masterfrom
jnm2:faster_resolver
Jan 5, 2020
Merged

Use stripped-down resolver for perf (no GAC, deferred reading mode)#54
sharwell merged 3 commits into
tunnelvisionlabs:masterfrom
jnm2:faster_resolver

Conversation

@jnm2

@jnm2 jnm2 commented Nov 9, 2019 •
edited
Loading

Copy link
Copy Markdown
Contributor

Contributes to #53.

Total assembly resolve time is 964 ms (version 1.0.0-alpha.99.g7d2f4a388d):

image

Where alpha.97 took 8328 ms:

image

Total time is now 17.4 seconds:

image

jnm2
jnm2 commented Nov 9, 2019
View reviewed changes
Comment thread TunnelVisionLabs.ReferenceAssemblyAnnotator/AssemblyResolver.cs Outdated
@jnm2
jnm2 force-pushed the faster_resolver branch 3 times, most recently from 1dc6eb7 to 7e3dcdc Compare November 9, 2019 01:58
@jnm2
jnm2 marked this pull request as ready for review November 9, 2019 02:18
@jnm2 jnm2 mentioned this pull request Nov 9, 2019
Open
@sharwell

sharwell commented Dec 30, 2019

Copy link
Copy Markdown
Member

@jnm2 this is currently failing during integration tests

@jnm2

jnm2 commented Dec 30, 2019

Copy link
Copy Markdown
Contributor Author

Nice!

@jnm2

jnm2 commented Dec 31, 2019

Copy link
Copy Markdown
Contributor Author

@sharwell On the master branch, Mono.Cecil is resolving mscorlib from the GAC as the core library for netstandard1.6 reference assemblies (e.g. microsoft.win32.primitives\4.3.0\ref\netstandard1.3\Microsoft.Win32.Primitives.dll).

The failure is due to this PR removing GAC loading which causes module.TypeSystem.Object.Resolve().Module to throw NRE instead of loading mscorlib from Framework64.

@jnm2

jnm2 commented Dec 31, 2019 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

I think the fix may require the target to resolve the netstandard1.6 core library (netstandard.dll?) and pass the path as a new parameter to the annotator. Going to get the targets PR merged first.

@jnm2
jnm2 force-pushed the faster_resolver branch 6 times, most recently from 6685c82 to ed08a08 Compare January 1, 2020 02:48
@jnm2

jnm2 commented Jan 1, 2020 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Failing with System.IO.DirectoryNotFoundException: Could not find a part of the path 'C:\projects\referenceassemblyannotator\tests\.isolatednugetcache\netstandard.library\2.0.3\build\netstandard2.0\ref\netstandard.dll'.

I verified locally that this happens to be fixed if I cherry-pick the first commit of #67.

@jnm2 jnm2 mentioned this pull request Jan 1, 2020
Merged
@jnm2
jnm2 force-pushed the faster_resolver branch from ed08a08 to bc2840b Compare January 4, 2020 16:23
@jnm2

jnm2 commented Jan 4, 2020

Copy link
Copy Markdown
Contributor Author

Rebased now that #67 is merged (rebasing wasn't needed, but I also wanted to retrigger CI)

@jnm2

jnm2 commented Jan 4, 2020

Copy link
Copy Markdown
Contributor Author

@sharwell CI passed.

@sharwell
sharwell merged commit c14d291 into tunnelvisionlabs:master Jan 5, 2020
@jnm2
jnm2 deleted the faster_resolver branch January 5, 2020 08:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@sharwell sharwell sharwell approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@jnm2 @sharwell