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 requires linker hints to preserve APIs accessed via reflection #10963

Closed
Kortdag opened this issue Feb 14, 2018 · 21 comments
Closed

Xamarin requires linker hints to preserve APIs accessed via reflection #10963

Kortdag opened this issue Feb 14, 2018 · 21 comments

Comments

@Kortdag
Copy link

Kortdag commented Feb 14, 2018

I had an issue using EF in a Xamarin app using a linker process.
According to the community the issue is due to EF using reflection internally which results in the linker removing framework parts that it should not.

Apprarently it can be solved by EF Core adding linker hints...
See
xamarin/xamarin-macios#3441 (comment)
xamarin/xamarin-macios#3394

So a suggestion to do so :-)

@divega
Copy link
Contributor

divega commented Feb 24, 2018

@spouliot A few questions about this:

  1. What is the recommended way to figure out what needs to be preserved, just trial an error? (ideally we would like to run our tests on Xamarin.iOS, but not sure that is possible). Or is there some guideline that can simplify the process?
  2. What PreserveAttribute do we need to use? Would we need to take a new dependency or can we define our own with the same shape?
  3. We had the experience solving similar problems for .NET Native, and it was a very tedious process (we would like to avoid doing again 😄), which resulted in our current rd.xml files. Do you happen to know how the requirements differ between .NET Native and your linker? Assuming the requirements are similar, could the linker consume rd.xml?

@divega
Copy link
Contributor

divega commented Feb 24, 2018

@spouliot one more thing about rd.xml: the whole platform has gone through the process of figuring out what should be preserved by default, e.g. see System.Linq.Expressions.rd.xml, so unless the requirements for the linker are different, it would be ideal of these could be leveraged.

@cwrea
Copy link

cwrea commented Mar 2, 2018

@divega On your point 2 above, the [PreserveAttribute] is matched by name and no dependency need be taken on Xamarin libraries.See Xamarin - iOS - Advanced Topics - Linking on iOS. Quoting:

If you do not want to take a dependency on the Xamarin libraries -for example, say that you are building a cross platform portable class library (PCL) - you can still use this attribute.

To do this, you should just declare a PreserveAttribute class, and use it in your code, like this:

public sealed class PreserveAttribute : System.Attribute {
    public bool AllMembers;
    public bool Conditional;
}

It does not really matter in which namespace this is defined, the linker looks this attribute by type name.

@cwrea
Copy link

cwrea commented Mar 2, 2018

@divega After some brainstorming, here's an idea related to your point 1 and your follow-up comment:

It might not strictly be necessary to run tests on Xamarin.iOS (although that would be ideal) to discover what needs preserving. The types and methods that Xamarin.iOS would need preserved in EF Core are probably the same ones that would need to be preserved for any target using a linker like that used by Xamarin.iOS.

And if I recall correctly, the linker used for Xamarin.iOS is descended from the Mono Linker project. Moreover, the .NET team has built a linker, the .NET IL Linker, based on the Mono Linker. And the .NET IL Linker has listed in its "Caveats" a point that could be a feature to help discover what needs to be preserved in EF Core:

The linker has the following caveats. [...]

  • The linker can and will break some apps that use reflection. See Using IL Linker Advanced Features for more information about managing reflection usage.

@divega
Copy link
Contributor

divega commented Mar 3, 2018

@cwrea thanks for the additional information. This makes standardizing on a single mechanism to specify what reflection metadata needs to be preserved even more compelling.

Still hoping to get an answer from @spuliot on the second and third questions.

@spouliot
Copy link

spouliot commented Mar 5, 2018

@divega

What is the recommended way to figure out what needs to be preserved, just trial an error? (ideally we would like to run our tests on Xamarin.iOS, but not sure that is possible).

Trial and error won't totally work. E.g.

  • That would assume 100% coverage; and even then

  • You can reflect Foo sucessfully because the application (or another library or even in your own library) has a reference on Foo. That might not be true for another application and the code will fail;

Running your tests on iOS would require a Mac/Xcode but it's not terribly hard to set up (simulator). However it will only catch what's tested, along with the accompanying code.

Or is there some guideline that can simplify the process?

Audit all of your usage of reflection. Make sure the types (minimally) or the methods (optimally) are preserved by an XML file, [Preserve] attributes or or by code. That's the only safe way to enable the linker on your code.

What PreserveAttribute do we need to use? Would we need to take a new dependency or can we define our own with the same shape?

You can define your own PreserveAttribute as long as it match the one from Xamarin.iOS (and other products using the linker). The namespace can differ too (it's a string-based comparison) only the name and properties really matter.

This was made (long ago) to avoid additional references on user (and 3rd party) code. The linker will remove them from the output (so it has no negative impact of linked application).

You can even add [assembly:LinkerSafe] to your assemblies once you have completed the audit work (that's another attribute you can add inside your own project). That will tell the linker to assume the assembly can be linked by default (just like any other SDK/BCL assemblies we ship with our package).

We had the experience solving similar problems for .NET Native, and it was a very tedious process (we would like to avoid doing again 😄), which resulted in our current rd.xml files. Do you happen to know how the requirements differ between .NET Native and your linker? Assuming the requirements are similar, could the linker consume rd.xml?

I have not used .NET Native so I can't be 100% sure - still it's the same challenges and there's not much latitude in solving them. It looks like the linked XML file could be transformed but it's not the same class libraries (e.g. System.Private.CoreLib) so it won't be a complete/perfect match.

However I know that .NET core is now using the same, base linker code as Xamarin (iOS, Mac and Android) products. Time invested in fixing this should prove useful to all of them.

@cwrea
Copy link

cwrea commented Mar 5, 2018

@spouliot Re: "Running your tests on iOS would require a Mac/Xcode but it's not terribly hard to set up (simulator)." Confession: I didn't know that builds targeting the simulator will actually use linking when so instructed. I'd just assumed the directive was ignored, but now I'm understanding "Don't Link" is merely a default. Anyway, great to know for diagnosing linking issues specifically ... I'd been doing a lot of that lately and now I'm kicking myself that I didn't know this! :)

But more broadly speaking, and I apologize for veering slightly off topic here (but I'll circle back): I made avoiding the simulator a habit due to another feature-slash-limitation. In the Xamarin iOS Architecture page, this: "If you are running [...] a Xamarin.iOS application on the simulator, the .NET Common Language Runtime (CLR) compiles the MSIL using a Just in Time (JIT) compiler." i.e. Simulator builds don't perform the AOT compilation that's necessary for real devices.

On the one hand, a time saver: You're tweaking UI code and want short edit-build-test cycles — great.

On the other hand, without AOT the simulator's value for testing plumbing code is reduced. False positives arise w.r.t. known limitations as I'm sure you're aware. In my own early quest for a lightweight, cross-platform ORM for use in both .NET web apps and Xamarin.iOS, more than once the simulator gave hope that AOT later snatched away, as the simulator was OK with performing Reflection.Emit().

So, despite having used the simulator for app development with Apple's tooling and languages, I decided to avoid it with Xamarin. I could accept obvious physical limitations (no camera, no GPS, etc.) would preclude simulator use for some feature scenarios, but IMHO even more generally I didn't want to use a tool that could so easily give false hope. "Fool me once..." etc.

Thankfully, Reflection.Emit() isn't in .NET Standard 2.0, and EF Core avoided it early for compatibility with UWP, with iOS benefitting too. But, there remains the possibility that other AOT limitations, and I'm thinking of those w.r.t. generics, could be problematic for .NET Standard libraries having non-trivial plumbing.

So what I'm trying to say is: Perhaps AOT should be an option one could enable in simulator builds? It would make the simulator more useful for .NET Standard library developers aiming for wider compatibility, and for app developers vetting external dependencies they're thinking of taking on.

Or, more generally: imagine tooling or a specialized runtime, both suitable for headless automated testing as well as where linking and AOT are the facts of life. If Xamarin/Mono/Microsoft had something like this available to library developers, whatever their development platform, it certainly wouldn't eliminate the need for proper testing on other operating systems and different CPU architectures. But it would go a long way in raising odds that .NET Standard library code developed with good intentions of being cross-platform would "just work" in the strictest environments.

@spouliot
Copy link

spouliot commented Mar 5, 2018

@cwrea

as the simulator was OK with performing Reflection.Emit()

Nope, at least not by default. You can enable REPL (optional) but otherwise the mscorlib.dll shipped with XI does not have support for SRE, including none of the types so you'll fail at build time.

other AOT limitations, and I'm thinking of those w.r.t. generics

To be honest the number of limitations are shrunk significally over the years and very few people hit them anymore. You're much more likely to hit sim/device differences - e.g. related to the amount of memory available.

Also the removal of some limitations did have a performance impact, e.g. sharing generic value types. However that's a case you're far more likely to notice if running on devices.

On the one hand, a time saver:

Yeah - it's not, by Apple design, an emulator and it shows in many places beside hardware capabilities (e.g. largely no sandbox in place).

It's very useful because it can be much faster... but for some type of development it does not help (anything the simulator does not support).

So YMMV with the simulator, but it's generally a pretty good tool but it cannot replace at least some testing on devices.

Perhaps AOT should be an option one could enable in simulator builds?

It's been discussed many times. However there's no scenario that really benefits from it.

  • It would slow down builds a lot - the main reason why the simulator is useful. It would not be any faster than current device builds;

  • It still won't be the same code running - it will be a different backend, x86_64, with its own quirks. It would also hide most performance/memory issues; and

  • It won't behave like the device hardware - because Apple does not give us an emulator.

So that would only add one more virtual platform to test and support. One that does not help most developer productivity, does not really help much compatibility and give you an incorrect feeling that things will be fine later. The reality is that you can't avoid some testing on devices.

My suggestion is that if/when you do not care about build times you're much better off testing on the real (device) thing.

If build times are an issue then take an approach similar to ours. We test every commit on the simulator (takes 4-5 hours). But we can't do that on device builds - it's just too slow and the test matrix is too big (it's more than 24h of tests per commit). So the simulator testing act as a filter: it catch most, not all issues, and we get daily updates from devices.

@divega divega changed the title Addding linker hints Xamarin requires linker hints to preserve reflection information Mar 24, 2018
@divega divega changed the title Xamarin requires linker hints to preserve reflection information Xamarin requires linker hints to preserve APIs accessed via reflection Mar 24, 2018
@divega divega removed this from the 2.1.0 milestone Mar 24, 2018
@divega
Copy link
Contributor

divega commented Mar 24, 2018

Clearing up milestone so that we can discuss this in triage.

Notes:

  <assembly fullname="mscorlib">
    <type fullname="System.String">
      <method name="Compare"></method>
      <method name="CompareTo"></method>
      <method name="ToUpper"></method>
      <method name="ToLower"></method>
    </type>
  </assembly>
  <assembly fullname="System.Core">
    <type fullname="System.Linq.Expressions.Expression`1"></type>
    <type fullname="System.Linq.Queryable"></type>
  </assembly>
</linker>

@cwrea
Copy link

cwrea commented Mar 24, 2018

@divega Thanks. A few FWIW's:

  1. The mention is appreciated. For completeness, the sample is adapted from this Xamarin sample described in the docs. I merely ported it to use EF Core instead of sqlite-net.

  2. The required types and methods in the LinkDescription snippet quoted were originally discovered with the sample app on EF Core v1.1.2, and for Xamarin.iOS. As you point out, these are not types from EF Core. I had linking set to "Link SDK assemblies only" — which I interpret as assemblies not in my project, and assemblies not from NuGet, even though much of what app developers would consider "SDK assemblies" are now delivered via NuGet. I presume the only assemblies subject to linking were Mono bits and Xamarin.iOS (but not Xamarin.Forms) from the local VS/Xamarin installation.

    For expediency, I assumed missing methods causing run-time crashes on iOS originally could remain issues even when moving EF Core v1.x to v2.x. So, while I believe the concept is sound, the specific examples from the sample may be technically redundant today.

    But, more recently I did try to get the iOS sample working with full linking, until I hit a blocker (point 3 below). Here's my LinkDescription work-in-progress up to that point, to get an idea of what I was dealing with from EF Core in particular:

    Actual EF Core types that get linked away.txt.

    Each line therein was a result of watching the sample crash with debugger attached, adding the missing type, rebuilding, and retesting. It looks like a lot, but not actually as fine-grained as it could be, as I'm instructing the linker to include the entire types mentioned, and not focusing only on constructors and the specific methods and properties required of those types.

  3. It's buried in the comment stream further above, but I'll call out one issue I posted to Xamarin.iOS: Xamarin.iOS linker removes assembly attributes (...). As consequence, even if EF Core itself had all the correct hints to work with Xamarin linking, it would (today) still fail at runtime under full linking on iOS because the AssemblyInformationalVersion that EF Core requires of its main assembly Microsoft.EntityFrameworkCore is removed by the Xamarin linker — no matter what.

    Clearing the milestone for now is a good idea as there may be no way to fully test this until the Xamarin linker provides the means to preserve assembly attributes. Alternatively, if EF Core were to retrieve its version from somewhere other than its own assembly attributes, full linking for EF Core wouldn't depend on that fix. And, Microsoft.EntityFrameworkCore assembly aside, there could still be some benefit in size reduction using full linking on supporting assemblies (Relational, Sqlite, etc.). But the main assembly is the big one.

@ajcvickers ajcvickers added this to the Backlog milestone Mar 28, 2018
@divega
Copy link
Contributor

divega commented Mar 29, 2018

Some notes on the triage decision we made today to move this to be backlog:

  1. We agree doing something in this area would be valuable and we want to do it.
  2. We need to do the due diligence to understand what is the best strategy and then execute on it. We see the recommendations above and we are hesitant to do something that would be brittle and that adds a maintainability burden, so we want to investigate further.
  3. We don't believe that there is much we can do for 2.1 given the schedule.
  4. In the meanwhile having some guidance on how to build your own LinkDescription.xml file (and having example starting points like the ones provided by @cwrea) or even using the linker options to avoid tree shaking on entire assemblies seems to be a reasonable mitigation.

@rspulini
Copy link

Is there any workaround?

I am using the latest versions of entity and Xamarin. In the Android project it works perfectly, but not in iOS.

Can you guide me? Thanks

@JeremyLikness
Copy link
Member

Revisiting this. @spouliot does adding the interpreter address this issue, or are you aware if there are still linking concerns even with the interpreter? @cwrea is your list of "linked away" types still valid? I know it's been awhile since that post. Basically I'm trying to figure out:

  1. Does documenting use of --interpreter close this issue? If not,
  2. Does documenting the <linker> directive with the files identified by @cwrea work?
  3. If not, more investigation ...

JeremyLikness added a commit to dotnet/EntityFramework.Docs that referenced this issue Sep 25, 2020
Documentation related to [dotnet/efcore #22559](dotnet/efcore#22559). May address [dotnet/efore #10963](dotnet/efcore#10963).
@ajcvickers ajcvickers modified the milestones: Backlog, 6.0.0 Oct 28, 2020
@bricelam
Copy link
Contributor

bricelam commented Oct 29, 2020

We should also check whether we can make the bundle initializer on Microsoft.Data.Sqlite work on iOS by using linker hints.

Update: It works now on .NET 5

@AndriySvyryd
Copy link
Member

#24903 would help with this if we are able to do it.

@roji
Copy link
Member

roji commented Aug 2, 2021

@JeremyLikness @bricelam as discussed in triage, it seems like we need to get the latest info for .NET 6.0, given the .NET unification and MAUI. I'm specifically interested in the following:

Any feedback on the above? Do you know the right people to ask about these things?

@ajcvickers
Copy link
Member

Note from triage: for 6.0 we will document where we are at using linkers, including [PreserveAttribute]. Other work here is covered by #21894, so closing this as duplicate of that issue and dotnet/EntityFramework.Docs#3377

@ajcvickers ajcvickers added closed-duplicate and removed type-enhancement area-perf help wanted This issue involves technologies where we are not experts. Expert help would be appreciated. area-xamarin labels Aug 12, 2021
@ajcvickers ajcvickers removed this from the 6.0.0 milestone Aug 12, 2021
@ajcvickers ajcvickers reopened this Oct 16, 2022
@ajcvickers ajcvickers closed this as not planned Won't fix, can't repro, duplicate, stale Oct 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests