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

[linker] Ensure we can remove NSUrlSessionHandler if unused #7151

Merged

Conversation

spouliot
Copy link
Contributor

@spouliot spouliot commented Oct 1, 2019

Moving NSUrlSessionHandler into the platform assemblies (e.g.
Xamarin.iOS.dll) instead of System.Net.Http.dll was not optimal as it
prevented the linker to remove it when the application did not use it.

This shows a lot in an "helloworld" type of application (see below)
but in real life most of the removed code gets used by something else
(and is included.

Directories / Files                                                              helloworld-d16-4.app helloworld-fixed.app         diff            %
./
        AppIcon60x60@2x.png                                                             2,632        2,632            0       0.00 %
        AppIcon76x76@2x~ipad.png                                                        3,125        3,125            0       0.00 %
        archived-expanded-entitlements.xcent                                              181          181            0       0.00 %
        Assets.car                                                                     75,688       75,688            0       0.00 %
        embedded.mobileprovision                                                        8,456        8,456            0       0.00 %
        helloworld                                                                  4,700,256    3,915,936     -784,320     -16.69 %
        helloworld.aotdata.arm64                                                        1,448        1,432          -16      -1.10 %
        helloworld.exe                                                                  6,144        6,144            0       0.00 %
        Info.plist                                                                      1,712        1,712            0       0.00 %
        mscorlib.aotdata.arm64                                                        406,080      364,104      -41,976     -10.34 %
        mscorlib.dll                                                                  561,664      501,760      -59,904     -10.67 %
        NOTICE                                                                            159          159            0       0.00 %
        PkgInfo                                                                             8            8            0       0.00 %
        System.aotdata.arm64                                                           17,008          936      -16,072     -94.50 %
        System.Core.aotdata.arm64                                                       2,432            0       -2,432    -100.00 %
        System.Core.dll                                                                 4,608            0       -4,608    -100.00 %
        System.dll                                                                     32,768        5,120      -27,648     -84.38 %
        System.Net.Http.aotdata.arm64                                                  31,648            0      -31,648    -100.00 %
        System.Net.Http.dll                                                            29,184            0      -29,184    -100.00 %
        Xamarin.iOS.aotdata.arm64                                                      62,544       33,464      -29,080     -46.50 %
        Xamarin.iOS.dll                                                                92,672       53,248      -39,424     -42.54 %
./_CodeSignature
        CodeResources                                                                   7,575        6,655         -920     -12.15 %
./LaunchScreen.storyboardc
        01J-lp-oVM-view-Ze5-6b-2t3.nib                                                  1,831        1,835            4       0.22 %
        Info.plist                                                                        258          258            0       0.00 %
        UIViewController-01J-lp-oVM.nib                                                   896          896            0       0.00 %
./Main.storyboardc
        BYZ-38-t0r-view-8bC-Xf-vdC.nib                                                  1,836        1,832           -4      -0.22 %
        Info.plist                                                                        258          258            0       0.00 %
        UIViewController-BYZ-38-t0r.nib                                                   916          916            0       0.00 %

Statistics

Native subtotal                                                                     4,700,256    3,915,936     -784,320     -16.69 %
    Executable                                                                      4,700,256    3,915,936     -784,320     -16.69 %
    AOT data *.aotdata                                                                      0            0            0          -

Managed *.dll/exe                                                                     727,040      566,272     -160,768     -22.11 %

TOTAL                                                                               6,053,987    4,986,755   -1,067,232     -17.63 %

Moving `NSUrlSessionHandler` into the platform assemblies (e.g.
Xamarin.iOS.dll) instead of System.Net.Http.dll was not optimal as it
prevented the linker to remove it when the application did not use it.

This shows a lot in an "helloworld" type of application (see below)
but in real life most of the removed code gets used by something else
(and is included.

```
Directories / Files                                                              helloworld-d16-4.app helloworld-fixed.app         diff            %
./
        AppIcon60x60@2x.png                                                             2,632        2,632            0       0.00 %
        AppIcon76x76@2x~ipad.png                                                        3,125        3,125            0       0.00 %
        archived-expanded-entitlements.xcent                                              181          181            0       0.00 %
        Assets.car                                                                     75,688       75,688            0       0.00 %
        embedded.mobileprovision                                                        8,456        8,456            0       0.00 %
        helloworld                                                                  4,700,256    3,915,936     -784,320     -16.69 %
        helloworld.aotdata.arm64                                                        1,448        1,432          -16      -1.10 %
        helloworld.exe                                                                  6,144        6,144            0       0.00 %
        Info.plist                                                                      1,712        1,712            0       0.00 %
        mscorlib.aotdata.arm64                                                        406,080      364,104      -41,976     -10.34 %
        mscorlib.dll                                                                  561,664      501,760      -59,904     -10.67 %
        NOTICE                                                                            159          159            0       0.00 %
        PkgInfo                                                                             8            8            0       0.00 %
        System.aotdata.arm64                                                           17,008          936      -16,072     -94.50 %
        System.Core.aotdata.arm64                                                       2,432            0       -2,432    -100.00 %
        System.Core.dll                                                                 4,608            0       -4,608    -100.00 %
        System.dll                                                                     32,768        5,120      -27,648     -84.38 %
        System.Net.Http.aotdata.arm64                                                  31,648            0      -31,648    -100.00 %
        System.Net.Http.dll                                                            29,184            0      -29,184    -100.00 %
        Xamarin.iOS.aotdata.arm64                                                      62,544       33,464      -29,080     -46.50 %
        Xamarin.iOS.dll                                                                92,672       53,248      -39,424     -42.54 %
./_CodeSignature
        CodeResources                                                                   7,575        6,655         -920     -12.15 %
./LaunchScreen.storyboardc
        01J-lp-oVM-view-Ze5-6b-2t3.nib                                                  1,831        1,835            4       0.22 %
        Info.plist                                                                        258          258            0       0.00 %
        UIViewController-01J-lp-oVM.nib                                                   896          896            0       0.00 %
./Main.storyboardc
        BYZ-38-t0r-view-8bC-Xf-vdC.nib                                                  1,836        1,832           -4      -0.22 %
        Info.plist                                                                        258          258            0       0.00 %
        UIViewController-BYZ-38-t0r.nib                                                   916          916            0       0.00 %

Statistics

Native subtotal                                                                     4,700,256    3,915,936     -784,320     -16.69 %
    Executable                                                                      4,700,256    3,915,936     -784,320     -16.69 %
    AOT data *.aotdata                                                                      0            0            0          -

Managed *.dll/exe                                                                     727,040      566,272     -160,768     -22.11 %

TOTAL                                                                               6,053,987    4,986,755   -1,067,232     -17.63 %
```
@monojenkins
Copy link
Collaborator

Build failure
Build succeeded
API Diff (from stable)
API Diff (from PR only) (no change)
Generator Diff (no change)
🔥 Test run failed 🔥

Test results

12 tests failed, 78 tests passed.

Failed tests

  • monotouch-test/iOS Unified 32-bits - simulator/Debug: Failed
  • monotouch-test/iOS Unified 32-bits - simulator/Debug (LinkSdk): Failed
  • monotouch-test/iOS Unified 32-bits - simulator/Debug (static registrar): Failed
  • monotouch-test/iOS Unified 32-bits - simulator/Release (all optimizations): Failed
  • monotouch-test/iOS Unified 64-bits - simulator/Debug: Failed
  • monotouch-test/iOS Unified 64-bits - simulator/Debug (LinkSdk): Failed
  • monotouch-test/iOS Unified 64-bits - simulator/Debug (static registrar): Failed
  • monotouch-test/iOS Unified 64-bits - simulator/Release (all optimizations): Failed
  • monotouch-test/tvOS - simulator/Debug: Failed
  • monotouch-test/tvOS - simulator/Debug (LinkSdk): Failed
  • monotouch-test/tvOS - simulator/Debug (static registrar): Failed
  • monotouch-test/tvOS - simulator/Release (all optimizations): Failed

@mandel-macaque
Copy link
Member

👍 but we need to make sure that UrlProtocolTest.RegistrarTest : DidReceiveData is not failing due to the change. First time I see this test fail.

@spouliot
Copy link
Contributor Author

spouliot commented Oct 2, 2019

Failures are fixed by #7154 and unrelated to the PR

@spouliot spouliot merged commit 82b84d0 into xamarin:master Oct 2, 2019
@spouliot spouliot deleted the remove-nsurlsessionhandler-if-unused branch October 2, 2019 14:10
@spouliot
Copy link
Contributor Author

spouliot commented Oct 3, 2019

@monojenkins backport to d16-4

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.

5 participants