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

[net8.0] Merge main into net8.0. #18436

Merged
merged 6 commits into from
Jun 13, 2023

Conversation

rolfbjarne
Copy link
Member

No description provided.

rolfbjarne and others added 6 commits June 8, 2023 16:51
…age back to Windows. Fixes xamarin#18402. (xamarin#18419)

This fixes a build problem on windows when a project has a project reference
to a binding project. The binding resource package from the binding project
would lack the manifest, and thus the main project wouldn't detect it as a
binding resource package, effectively not seeing any native resource from the
binding project.

Fixes xamarin#18402.
…ies (xamarin#18408)

# Description

This PR reduces the application's SOD (size on disk) by making
`__LINKEDIT Export Info` section smaller in the stripped Mach-O
binaries.

The feature is controlled by `_ExportSymbolsExplicitly` MSBuild property
and can be disabled by specifying: `-p:_ExportSymbolsExplicitly=true`

Fixes xamarin#18332 

# Initial problem

It has been noticed that during stripping, the strip tool does not
resize the export info section after it removes the symbols. Instead it
only zeroes out the entries (achieved by calling `prune_trie` function):

- https://github.com/apple-oss-distributions/cctools/blob/cctools-986/misc/strip.c
- https://github.com/apple-oss-distributions/ld64/blob/ld64-711/src/other/PruneTrie.cpp

Thanks @lambdageek for helping to track this down.

# Approach

As Xamarin build process already collects all the [required symbols][1] needed
for the application to run and preserves them during the strip phase, we can
use the same file to instruct clang toolchain to export only those symbols via
the command line options: `-exported_symbols_list <file>` ([source][2]).

This will make the export info section only include what is necessary for the
runtime - and at the same time eliminate the problem of the `strip` tool which
does not resize stripped symbols.

# Investigation setup

The issue is observable by building and inspecting the test application:
https://github.com/xamarin/xamarin-macios/blob/main/tests/dotnet/MySingleView/MySingleView.csproj
and targeting iOS platform in Release mode.

## Results:

| Measure      | MySingleView - main | MySingleView - this PR | Diff (%) | 
| :---         |                ---: |                   ---: |     ---: |
| SOD (bytes)  |            13668940 |               13458476 |    -1.5% |
| .ipa (bytes) |             4829368 |                4827928 |   -0.03% |

Even though zeroes are compressed well, the SOD is still affected and
unused section takes around 1.5% of the most simplistic app size.
Much bigger impact has been noted when trying out a MAUI iOS template
app with NativeAOT where the `__LINKEDIT Export Info` zeroes take up to
20MB of the SOD, but also with the regular macOS applications:
dotnet/runtime#86707

### Repro current state of MySingleView.app with stripped binary

1. Build the app (you can ignore the need to run the sample, I just did it to
   make sure the changes do not break anything)

```bash
make run-device
``` 

2. Print the load commands - [load_cmds_strip.list][3]

```bash
otool -l bin/Release/net7.0-ios/ios-arm64/MySingleView.app/MySingleView > load_cmds_strip.list
```

- We are interested in the export info section:

```
cmd LC_DYLD_INFO_ONLY
...
export_off 5942960
export_size 207712
```

3. Create a hex dump of the export info section - [hex_dump_strip.list][4]

``` bash
xxd -s 5942960 -l 207712 bin/Release/net7.0-ios/ios-arm64/MySingleView.app/MySingleView > hex_dump_strip.list
```

- NOTE: Notice around ~200kb of zeroes from ~0x005ab490 to ~0x005dda00
    
4. Verify exported symbols are correct - [dyld_info_strip.list][5]

``` bash
dyld_info -exports bin/Release/net7.0-ios/ios-arm64/MySingleView.app/MySingleView > dyld_info_strip.list
```

### Repro current state of MySingleView.app with unstripped binary

1. Build the app (the make target preserves the symbols)

```bash
make run-device-no-strip
``` 

2. Print the load commands - [load_cmds_nostrip.list][6]

```bash
otool -l bin/Release/net7.0-ios/ios-arm64/MySingleView.app/MySingleView > load_cmds_nostrip.list
```

- We are interested in the export info section:

```
cmd LC_DYLD_INFO_ONLY
...
export_off 5942960
export_size 207712
```

3. Create a hex dump of the export info section - [hex_dump_nostrip.list][7]

``` bash
xxd -s 5942960 -l 207712 bin/Release/net7.0-ios/ios-arm64/MySingleView.app/MySingleView > hex_dump_nostrip.list
```

- Notice that the range: ~ 0x005ab490 to ~ 0x005dda00 now includes exported symbol entries
    
4. Verify exported symbols are correct - [dyld_info_nostrip.list][8]

``` bash
dyld_info -exports bin/Release/net7.0-ios/ios-arm64/MySingleView.app/MySingleView > dyld_info_nostrip.list
```

### Repro the new approach 

1. Build the app (the make target uses the new approach)

```bash
make run-device-export-syms
``` 

2. Print the load commands - [load_cmds_export.list][9]

```bash
otool -l bin/Release/net7.0-ios/ios-arm64/MySingleView.app/MySingleView > load_cmds_export.list
```

- We are interested in the export info section ***notice the reduced size of the section***:

```
cmd LC_DYLD_INFO_ONLY
...
export_off 5942432
export_size 1048
```

3. Create a hex dump of the export info section - [hex_dump_export.list][10]

``` bash
xxd -s 5942432 -l 1048 bin/Release/net7.0-ios/ios-arm64/MySingleView.app/MySingleView > hex_dump_export.list
```
    
4. Verify exported symbols are correct - [dyld_info_export.list][11]

``` bash
dyld_info -exports bin/Release/net7.0-ios/ios-arm64/MySingleView.app/MySingleView > dyld_info_export.list
```

---

## Additional benefits

With this approach we could also switch the way strip tool is invoked to
always strip all debug and local symbols via `strip -S -x` instead of passing
the file with symbols to preserve. This would remove the warning that we are
currently getting (which is being ignored):

```
/Applications/Xcode_14.3.0.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/strip: warning: removing global symbols from a final linked no longer supported.  Use -exported_symbols_list at link time when building...
```

## Other references:

- https://github.com/qyang-nj/llios/blob/main/exported_symbol/README.md

[1]: https://github.com/xamarin/xamarin-macios/blob/11e7883da04d80c59e4ffbbc955a3e0e0060ff90/tools/dotnet-linker/Steps/GenerateReferencesStep.cs#L38-L44
[2]: https://developer.apple.com/library/archive/documentation/DeveloperTools/Conceptual/DynamicLibraries/100-Articles/DynamicLibraryDesignGuidelines.html
[3]: https://gist.github.com/ivanpovazan/d53f8d10be5e4ea9f39a41ea540aa7fa
[4]: https://gist.github.com/ivanpovazan/60637422f3ff8cb5f437ddd06a21d9c1
[5]: https://gist.github.com/ivanpovazan/352595ad15c2ac02f38dcb3bd4130642
[6]: https://gist.github.com/ivanpovazan/bf700161f2f3691d1d7381c98d4fa0be
[7]: https://gist.github.com/ivanpovazan/44269e4fff5ebd58a4d181451e5c106f
[8]: https://gist.github.com/ivanpovazan/38c5afe076502d514a77420af0e10b01
[9]: https://gist.github.com/ivanpovazan/3f663c3c630005f5a578605d48ba807e
[10]: https://gist.github.com/ivanpovazan/0bb84f64281d05ab20438aeaed64f13c
[11]: https://gist.github.com/ivanpovazan/78b3ba2288f53a2316b9bc46964e7e4f

---------

Co-authored-by: Rolf Bjarne Kvinge <rolf@xamarin.com>
…commit changes. (xamarin#18426)

This fixes a frequent issue where building locally again would create
test apps where the registrar would fail due to mismatched runtime versions.
@rolfbjarne rolfbjarne requested review from emaf and mauroa as code owners June 12, 2023 16:56
@vs-mobiletools-engineering-service2
Copy link
Collaborator

❌ [CI Build] Windows Integration Tests failed ❌

❌ Failed ❌

Pipeline on Agent
Hash: 5d6a92eaf0afd28b533400cddeb89335a3ab69b8 [PR build]

@vs-mobiletools-engineering-service2
Copy link
Collaborator

📚 [PR Build] Artifacts 📚

Packages generated

View packages

Pipeline on Agent
Hash: [PR build]

@vs-mobiletools-engineering-service2
Copy link
Collaborator

❌ [PR Build] Tests on macOS M1 - Mac Big Sur (11.5) failed ❌

Failed tests are:

  • dontlink
  • introspection
  • linksdk
  • linkall
  • monotouch-test

Pipeline on Agent
Hash: [PR build]

@vs-mobiletools-engineering-service2
Copy link
Collaborator

✅ API diff for current PR / commit

Legacy Xamarin (No breaking changes)
  • iOS (no change detected)
  • tvOS (no change detected)
  • watchOS (no change detected)
  • macOS (no change detected)
NET (empty diffs)
  • iOS: (empty diff detected)
  • tvOS: (empty diff detected)
  • MacCatalyst: (empty diff detected)
  • macOS: (empty diff detected)

✅ API diff vs stable

Legacy Xamarin (No breaking changes)
.NET (No breaking changes)
Legacy Xamarin (stable) vs .NET

ℹ️ Generator diff

Generator Diff: vsdrops (html) vsdrops (raw diff) gist (raw diff) - Please review changes)

Pipeline on Agent
Hash: 5d6a92eaf0afd28b533400cddeb89335a3ab69b8 [PR build]

@vs-mobiletools-engineering-service2
Copy link
Collaborator

❌ [PR Build] Tests on macOS M1 - Mac Ventura (13.0) failed ❌

Failed tests are:

  • dontlink
  • introspection
  • linksdk
  • linkall
  • monotouch-test

Pipeline on Agent
Hash: [PR build]

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2
Copy link
Collaborator

🚀 [CI Build] Test results 🚀

Test results

✅ All tests passed on VSTS: simulator tests.

🎉 All 229 tests passed 🎉

Tests counts

✅ bcl: All 69 tests passed. Html Report (VSDrops) Download
✅ cecil: All 1 tests passed. Html Report (VSDrops) Download
✅ dotnettests: All 1 tests passed. Html Report (VSDrops) Download
✅ fsharp: All 7 tests passed. [attempt 2] Html Report (VSDrops) Download
✅ framework: All 8 tests passed. Html Report (VSDrops) Download
✅ generator: All 2 tests passed. Html Report (VSDrops) Download
✅ interdependent_binding_projects: All 7 tests passed. Html Report (VSDrops) Download
✅ install_source: All 1 tests passed. Html Report (VSDrops) Download
✅ introspection: All 8 tests passed. [attempt 2] Html Report (VSDrops) Download
✅ linker: All 65 tests passed. Html Report (VSDrops) Download
✅ mac_binding_project: All 1 tests passed. Html Report (VSDrops) Download
✅ mmp: All 2 tests passed. Html Report (VSDrops) Download
✅ mononative: All 6 tests passed. Html Report (VSDrops) Download
✅ monotouch: All 35 tests passed. Html Report (VSDrops) Download
✅ msbuild: All 2 tests passed. Html Report (VSDrops) Download
✅ mtouch: All 1 tests passed. Html Report (VSDrops) Download
✅ xammac: All 3 tests passed. Html Report (VSDrops) Download
✅ xcframework: All 8 tests passed. Html Report (VSDrops) Download
✅ xtro: All 2 tests passed. Html Report (VSDrops) Download

Pipeline on Agent
Hash: 5d6a92eaf0afd28b533400cddeb89335a3ab69b8 [PR build]

@rolfbjarne rolfbjarne merged commit b0fba63 into xamarin:net8.0 Jun 13, 2023
@rolfbjarne rolfbjarne deleted the bump-main-in-net8.0-2023-06-12 branch June 13, 2023 08:53
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