Skip to content

Conversation

@radical
Copy link
Member

@radical radical commented Jan 5, 2021

  • The ConfigureTrimming target adds xunit assemblies explicitly, and
    sets TriggerRootAssembly too.

  • Looking at the files being linked, some issues noticed:

    • Multiple copies of xunit assemblies, from various locations are
      included. Eg. from System.Buffer.Tests, WasmTestRunner, and
      from the nuget
    • WasmTestRunner is explicitly added as TriggerRootAssembly, but
      it is also passed as an arg for linking, -p link ...
    • xunit.runner.json is incorrectly added to the files to be linked
  • Instead:

    • all the files to be published are already in
      ResolvedFileToPublish after AddTestRunnersToPublishedFiles target

    • so, we mark everything for linking,

    • except the main test assembly

    • Also, we use the test runner as a root

    • This way we are explicitly specifying which assemblies to link,
      and which ones to copy

  • This reduces the size:

    • 3.1M to 2.6M artifacts/bin/System.Buffers.Tests/net6.0-Release/browser-wasm/AppBundle/managed/
    • 46M to 31M artifacts/bin/System.Buffers.Tests/net6.0-Release/browser-wasm/AppBundle/publish/

- The `ConfigureTrimming` target adds xunit assemblies explicitly, and
  sets `TriggerRootAssembly` too.

- Looking at the files being linked, some issues noticed:
    - Multiple copies of xunit assemblies, from various locations are
      included. Eg. from `System.Buffer.Tests`, `WasmTestRunner`, and
      from the nuget
    - `WasmTestRunner` is explicitly added as `TriggerRootAssembly`, but
      it is also passed as an arg for linking, `-p link ...`
    - `xunit.runner.json` is incorrectly added to the files to be linked

- Instead:
    - all the files to be published are already in
      `ResolvedFileToPublish` after `AddTestRunnersToPublishedFiles` target
    - so, we mark everything for linking,
    - *except*:
        - main test assembly
        - and the test runner assembly

    - This way we are explicitly specifying which assemblies to link,
      and which ones to `copy`

- This reduces the size:
    - 3.1M to 2.6M `artifacts/bin/System.Buffers.Tests/net6.0-Release/browser-wasm/AppBundle/managed/`
    - 46M to 31M `artifacts/bin/System.Buffers.Tests/net6.0-Release/browser-wasm/AppBundle/publish/`
@ghost
Copy link

ghost commented Jan 5, 2021

Tagging subscribers to 'arch-wasm': @lewing
See info in area-owners.md if you want to be subscribed.

Issue Details
  • The ConfigureTrimming target adds xunit assemblies explicitly, and
    sets TriggerRootAssembly too.

  • Looking at the files being linked, some issues noticed:

    • Multiple copies of xunit assemblies, from various locations are
      included. Eg. from System.Buffer.Tests, WasmTestRunner, and
      from the nuget
    • WasmTestRunner is explicitly added as TriggerRootAssembly, but
      it is also passed as an arg for linking, -p link ...
    • xunit.runner.json is incorrectly added to the files to be linked
  • Instead:

    • all the files to be published are already in
      ResolvedFileToPublish after AddTestRunnersToPublishedFiles target

    • so, we mark everything for linking,

    • except:

      • main test assembly
      • and the test runner assembly
    • This way we are explicitly specifying which assemblies to link,
      and which ones to copy

  • This reduces the size:

    • 3.1M to 2.6M artifacts/bin/System.Buffers.Tests/net6.0-Release/browser-wasm/AppBundle/managed/
    • 46M to 31M artifacts/bin/System.Buffers.Tests/net6.0-Release/browser-wasm/AppBundle/publish/
Author: radical
Assignees: -
Labels:

arch-wasm, area-Infrastructure-libraries

Milestone: -

@ghost
Copy link

ghost commented Jan 5, 2021

Tagging subscribers to this area: @safern, @ViktorHofer
See info in area-owners.md if you want to be subscribed.

Issue Details
  • The ConfigureTrimming target adds xunit assemblies explicitly, and
    sets TriggerRootAssembly too.

  • Looking at the files being linked, some issues noticed:

    • Multiple copies of xunit assemblies, from various locations are
      included. Eg. from System.Buffer.Tests, WasmTestRunner, and
      from the nuget
    • WasmTestRunner is explicitly added as TriggerRootAssembly, but
      it is also passed as an arg for linking, -p link ...
    • xunit.runner.json is incorrectly added to the files to be linked
  • Instead:

    • all the files to be published are already in
      ResolvedFileToPublish after AddTestRunnersToPublishedFiles target

    • so, we mark everything for linking,

    • except:

      • main test assembly
      • and the test runner assembly
    • This way we are explicitly specifying which assemblies to link,
      and which ones to copy

  • This reduces the size:

    • 3.1M to 2.6M artifacts/bin/System.Buffers.Tests/net6.0-Release/browser-wasm/AppBundle/managed/
    • 46M to 31M artifacts/bin/System.Buffers.Tests/net6.0-Release/browser-wasm/AppBundle/publish/
Author: radical
Assignees: -
Labels:

arch-wasm, area-Infrastructure-libraries

Milestone: -

Copy link
Member

@mdh1418 mdh1418 left a comment

Choose a reason for hiding this comment

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

Thanks! Looks good to me.

Copy link
Member

@steveisok steveisok left a comment

Choose a reason for hiding this comment

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

Excellent! Thanks for trimming even more.

@radical radical requested a review from marek-safar January 5, 2021 17:21
Copy link
Member

@lewing lewing left a comment

Choose a reason for hiding this comment

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

👍

@radical radical merged commit 5f984ae into dotnet:master Jan 5, 2021
@radical radical deleted the trimming-changes branch January 5, 2021 22:50
@ghost ghost locked as resolved and limited conversation to collaborators Feb 4, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants