Repository navigation
Remove GetICUVersion from being preserved unconditionally by the ILLinker #39102
Description
We have one entry in CoreLib's ILLink.Descriptors.xml that is only used for testing:
We should remove this entry so the method is trimmed when linking an app. See @marek-safar's comment here: #35199 (comment)
Marking it all the time will cause problems for us in InvariantMode when the native part won't be available and we need to statically link it.
One option would be to make a "_LibraryBuild.xml" file for CoreLib and only preserve this method when doing the "library build". The descriptor entry won't be embedded into the assembly, so it won't be picked up when linking the user's application.
Activity
tarekgh commentedon Jul 10, 2020
I am wondering how much we'll gain when trimming this method? is it really worth it?
eerhardt commentedon Jul 10, 2020
It isn't so much about the size gain. The size gain is super minimal.
The bigger issue is the static linking with InvariantMode, as mentioned above.
jkotas commentedon Jul 10, 2020
So can we make it conditional on InvariantMode?
marek-safar commentedon Jul 10, 2020
What is the motivation to have this pinvoke compiled into SPC at all? My understanding is that this is used for some test checks only, why the pinvoke cannot be defined in test assembly directly?
tarekgh commentedon Jul 10, 2020
This was discussed when we introduced using ICU on Windows. The reason we preferred having it there is because the test assembly not aware if the ICU is loaded at all in the process or not or we are in NLS mode or ICU mode. Or if we are in NLS mode and somehow someone loaded ICU in the process for any other reason. having it in the SPC make it just straight forward if we are really in ICU mode without any heuristics.
safern commentedon Jul 10, 2020
Also, the reason why this PInvoke wasn't added into the test assembly directly is because the PInvoke is a QCall, which is internal and only allowed from
System.Private.CoreLibsinceSystem.Globalization.Nativeshim is linked into mono and coreclr.marek-safar commentedon Jul 12, 2020
I see. I agree with @eerhardt that the simplest solution then is to move the rule to "_LibraryBuild.xml" file for SPC.
eerhardt commentedon Jul 30, 2020
This work isn't necessary to ship 5.0, so moving to 6.0.
eerhardt commentedon Feb 16, 2021
Resolved by #48120.