[XamlC] no longer use any reflection-base ImportReference #1899

StephaneDelcroix · 2018-02-16T14:22:17Z

Description of Change

[XamlC] no longer use any reflection-base ImportReference

reflection-base ImportReference -- we were using MethodBase and Type --
are importing the types present in the reflection context, at the time
of compilation. As the compilation happens on netstandard2.0, and our
assembly can now be netstandard1.0 again, those imported types are
failing to be resolved.

this changes always import references based on the assembly, or the
assembly references.

it might, or might not, give us another speed bump.

PR Checklist

Has tests (if omitted, state reason in description)
Rebased on top of master at time of PR
Changes adhere to coding standard
Consolidate commits as makes sense

reflection-base ImportReference -- we were using MethodBase and Type -- are importing the types present in the reflection context, at the time of compilation. As the compilation happens on netstandard2.0, and our assembly can now be netstandard1.0 again, those imported types are failing to be resolved. this changes always import references based on the assembly, or the assembly references. it might, or might not, give us another speed bump.

jassmith · 2018-02-22T08:57:06Z

Will get you benchmark times tomorrow

PureWeen · 2018-02-23T18:44:14Z

For the latest PR
I have this version installed

<PackageReference Include="Xamarin.Forms" Version="2.6.0.210032" />

and with that I'm able to compile netstandard 1.4 with the following build times

1>      124 ms  ResolveAssemblyReference                   1 calls
1>      137 ms  JoinItems                                  8 calls
1>      186 ms  Copy                                       2 calls
1>      232 ms  Csc                                        1 calls
1>     9808 ms  XamlCTask                                  1 calls

Against netstandard 2.0 I have the following build times

1>      161 ms  ResolveAssemblyReference                   1 calls
1>      183 ms  Copy                                       2 calls
1>      203 ms  JoinItems                                  8 calls
1>      990 ms  Csc                                        1 calls
1>     1290 ms  XamlCTask                                  1 calls

samhouts · 2018-02-23T18:52:59Z

Xamarin.Forms.Build.Tasks/ModuleDefinitionExtensions.cs

+			//I hate you, netstandard
+			if (type.assemblyName == "mscorlib" && type.clrNamespace == "System.Reflection")
+				return module.GetTypeDefinition(("System.Reflection", type.clrNamespace, type.typeName));
+				throw new Exception($"Failed to get typedef for {type}");


👮 This LOOKS like this exception happens as part of this if clause because of the indentation.

StephaneDelcroix force-pushed the xamlc_noreflection branch 2 times, most recently from 03c9073 to ca121ef Compare February 16, 2018 14:26

StephaneDelcroix requested review from samhouts and jassmith February 16, 2018 14:35

jassmith approved these changes Feb 16, 2018

View reviewed changes

StephaneDelcroix added DO-NOT-MERGE-!!! 🛑 This is in progress and needs to be updated before it can be merged. and removed DO-NOT-MERGE-!!! 🛑 This is in progress and needs to be updated before it can be merged. labels Feb 16, 2018

StephaneDelcroix added 2 commits February 21, 2018 21:19

[XamlC] accomodate all kinds of netstandard versions

d73815f

StephaneDelcroix force-pushed the xamlc_noreflection branch from 423d2bb to d73815f Compare February 21, 2018 20:20

StephaneDelcroix changed the base branch from master to 3.0.0 February 22, 2018 08:06

jassmith approved these changes Feb 22, 2018

View reviewed changes

StephaneDelcroix mentioned this pull request Feb 23, 2018

use of XamlCompilation attribute leads to 'unable to resolve mscorlib' compliation warning #1958

Closed

samhouts approved these changes Feb 23, 2018

View reviewed changes

reuse caching code

e60c4cf

rmarinho merged commit 463714b into 3.0.0 Feb 26, 2018

pici-masp mentioned this pull request Mar 20, 2018

There was a conflict between mscorlib 4.0.0.0 and 2.0.5.0 UXDivers/Grial-UI-Kit-Support#389

Closed

samhouts added this to the 3.0.0 milestone May 5, 2018

StephaneDelcroix deleted the xamlc_noreflection branch May 8, 2018 07:31

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[XamlC] no longer use any reflection-base ImportReference #1899

[XamlC] no longer use any reflection-base ImportReference #1899

Uh oh!

StephaneDelcroix commented Feb 16, 2018 •

edited by samhouts

Loading

Uh oh!

jassmith commented Feb 22, 2018

Uh oh!

PureWeen commented Feb 23, 2018 •

edited

Loading

Uh oh!

samhouts Feb 23, 2018

Uh oh!

Uh oh!

[XamlC] no longer use any reflection-base ImportReference #1899

[XamlC] no longer use any reflection-base ImportReference #1899

Uh oh!

Conversation

StephaneDelcroix commented Feb 16, 2018 • edited by samhouts Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description of Change

PR Checklist

Uh oh!

jassmith commented Feb 22, 2018

Uh oh!

PureWeen commented Feb 23, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

samhouts Feb 23, 2018

Choose a reason for hiding this comment

Uh oh!

Uh oh!

StephaneDelcroix commented Feb 16, 2018 •

edited by samhouts

Loading

PureWeen commented Feb 23, 2018 •

edited

Loading