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

Use An AOT-friendly Linq implementation #894

Merged
merged 1 commit into from
Mar 30, 2018

Conversation

joshpeterson
Copy link

@joshpeterson joshpeterson commented Mar 29, 2018

The corefx implementation of Linq is not AOT-friendly, and won't work
with IL2CPP until IL2CPP gets full generic sharing support. So use the
Linq implementation from reference source for the unityaot profile. This
implementation is AOT-friendly.

This change addresses case 1013854 in Unity.

I'll add release notes with the IL2CPP changes for this issue.

@joshpeterson
Copy link
Author

I'm still waiting for our Mono build to complete with these changes.

@joshpeterson
Copy link
Author

The Mono build succeeded now.

@@ -2829,4 +2833,49 @@ public object[] Items
[System.Diagnostics.DebuggerBrowsable(System.Diagnostics.DebuggerBrowsableState.Never)]
private int count;
}

#if UNITY
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this needed for the unityjit profile? UNITY is defined for both unityjit and unityaot profiles. Where as UNITY_JIT and UNITY_AOT are the profile specific defines

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not needed for unityjit, but this .cs file is not included in the unityjit build. I was unaware that we had a specific define for unityaot. I'll change it to use that instead to be explicit.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've corrected this to use UNITY__AOT now.

@@ -1 +1,3 @@
#include winaot_System.Core.dll.sources
../referencesource/System.Core/System/Linq/Enumerable.cs
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We exclude all of Linq directory and only add Enumerable.cs?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that everything we need is in Enumerable.cs. In the corefx code, everything is broken up into smaller files in a partial class. I did test this code with our .NET Standard 2.0 compliance tests, and it passes. So I think this is correct.

The corefx implementation of Linq is not AOT-friendly, and won't work
with IL2CPP until IL2CPP gets full generic sharing support. So use the
Linq implementation from reference source for the unityaot profile. This
implementation is AOT-friendly.

This change addresses case 1013854 in Unity.
@joshpeterson joshpeterson force-pushed the linq-unityaot-profile branch from fd596b9 to dab14a9 Compare March 30, 2018 12:42
@joshpeterson joshpeterson merged commit 83e3daa into unity-master Mar 30, 2018
@joshpeterson joshpeterson deleted the linq-unityaot-profile branch March 30, 2018 12:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants