Skip to content

Remove GenericArraySortHelper from ILLinkTrim xml#37286

Merged
joperezr merged 3 commits intodotnet:masterfrom
joperezr:GenericArraySortHelper
Jun 4, 2020
Merged

Remove GenericArraySortHelper from ILLinkTrim xml#37286
joperezr merged 3 commits intodotnet:masterfrom
joperezr:GenericArraySortHelper

Conversation

@joperezr
Copy link
Copy Markdown
Member

@joperezr joperezr commented Jun 1, 2020

Remove the GenericArraySortHelper from ILLinkTrim and instead move them into attributes where they are being used.

@ghost
Copy link
Copy Markdown

ghost commented Jun 1, 2020

Tagging subscribers to this area: @ViktorHofer
Notify danmosemsft if you want to be subscribed.

@joperezr
Copy link
Copy Markdown
Member Author

joperezr commented Jun 1, 2020

I have validated that if I trim a simple Console app that calls Array.Sort methods these don't get trimmed.

@MichalStrehovsky @sbomer if I remove the Array.Sort calls I was expecting the .ctor of those types to get trimmed but that wasn't the case, and when I analyzed the dumped report I got that the linker kept them because:

--- TypeDef:System.Collections.Generic.GenericArraySortHelper`1:System.Private.CoreLib.dll dependencies ---
Dependency #1
        TypeDef:System.Collections.Generic.GenericArraySortHelper`1:System.Private.CoreLib.dll
        | TypeSpec:System.Collections.Generic.GenericArraySortHelper`1<System.String>:System.Private.CoreLib.dll [1 deps]
        | Method:System.Collections.Generic.IArraySortHelper`1<T> System.Collections.Generic.ArraySortHelper`1::CreateArraySortHelper() [1 deps]
        | MemberRef:System.Collections.Generic.IArraySortHelper`1<T> System.Collections.Generic.ArraySortHelper`1<T>::CreateArraySortHelper() [1 deps]
        | Method:System.Void System.Collections.Generic.ArraySortHelper`1::.cctor() [1 deps]
        | Field:System.Collections.Generic.IArraySortHelper`1<T> System.Collections.Generic.ArraySortHelper`1::s_defaultArraySortHelper [1 deps]
        | MemberRef:System.Collections.Generic.IArraySortHelper`1<T> System.Collections.Generic.ArraySortHelper`1<T>::s_defaultArraySortHelper [2 deps]
        | Method:System.Collections.Generic.IArraySortHelper`1<T> System.Collections.Generic.ArraySortHelper`1::get_Default() [1 deps]
        | MemberRef:System.Collections.Generic.IArraySortHelper`1<T> System.Collections.Generic.ArraySortHelper`1<T>::get_Default() [2 deps]
        | Method:System.Int32 System.Array::BinarySearch(T[],System.Int32,System.Int32,T,System.Collections.Generic.IComparer`1<T>) [1 deps]
        | MethodSpec:System.Int32 System.Array::BinarySearch<T>(T[],System.Int32,System.Int32,T,System.Collections.Generic.IComparer`1<T>) [1 deps]
        | Method:System.Int32 System.Array::BinarySearch(T[],T) [1 deps]
        | MethodSpec:System.Int32 System.Array::BinarySearch<System.UInt64>(T[],T) [3 deps]
        | Method:System.Boolean System.RuntimeType::IsEnumDefined(System.Object) [1 deps]
        | TypeDef:System.RuntimeType:System.Private.CoreLib.dll [456 deps]
        | Method:System.String System.Enum::ToString() [1 deps]
        | TypeDef:System.Enum [272 deps]
        | TypeDef:System.Diagnostics.DebuggableAttribute/DebuggingModes [8 deps]
        | Method:System.Void System.Diagnostics.DebuggableAttribute::.ctor(System.Diagnostics.DebuggableAttribute/DebuggingModes)

The wierd part here, is that if I remove the attributes I'm adding in this PR, and don't use Array.Sort, I would have expected the .ctor to still be kept as found by the report above given there is still something rooting it, but the .ctor is actually not present. Is this by design?

@MichalStrehovsky
Copy link
Copy Markdown
Member

The wierd part here, is that if I remove the attributes I'm adding in this PR, and don't use Array.Sort, I would have expected the .ctor to still be kept as found by the report above given there is still something rooting it, but the .ctor is actually not present. Is this by design?

Type is getting rooted by the typeof - linker cannot remove a type referenced statically by a typeof, but the members on the type are fair game.

The PreserveDependency ensures the constructor is kept (even though the code as-is won't actually use the ctor because RuntimeTypeHandle.Allocate below actually allocates a new instance without running a constructor - but keeping a constructor forces linker to keep a couple other things on the class that are actually important - I wish we had a "pretend this type was allocated" annotation).

@ghost
Copy link
Copy Markdown

ghost commented Jun 2, 2020

Hello @joperezr!

Because this pull request has the auto-merge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (@msftbot) and give me an instruction to get started! Learn more here.

@joperezr joperezr merged commit 49a245e into dotnet:master Jun 4, 2020
@joperezr joperezr deleted the GenericArraySortHelper branch June 4, 2020 17:16
@ghost ghost locked as resolved and limited conversation to collaborators Dec 9, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants