NativeAOT: Disable AggressiveAttributeTrimming with ILLink - #18545

Merged
dalexsoto merged 5 commits into
dotnet:net8.0from
ivanpovazan:naot-trim-enable
Jul 17, 2023
Merged

NativeAOT: Disable AggressiveAttributeTrimming with ILLink#18545
dalexsoto merged 5 commits into
dotnet:net8.0from
ivanpovazan:naot-trim-enable

Conversation

@ivanpovazan

@ivanpovazanivanpovazan commented Jul 10, 2023•
edited
Loading

Copy link
Copy Markdown
Member

In the current setup with NativeAOT, during app build we run both ILLink and ILCompiler in cascade.
When _AggressiveAttributeTrimming feature switch is set to true ILLink removes IsTrimmable attribute from assemblies, which means that when we are also running in TrimMode=partial (which translates to --defaultrooting ILC command-line arguments) ILC trimming is disabled completely.

This PR disables _AggressiveAttributeTrimming in the first pass ie during trimming by ILLink and enables it in the second trimming pass performed by ILCompiler.

Additionally, to workaround ILCompiler incompatibility with Microsoft.iOS (and friends) this platform assembly is explicitly rooted when passed to ILCompiler for trimming (this will be fixed once dotnet/runtime#86649 is resolved).

Estimated savings: This change reduces the size of the application bundle by 0,58Mb (or ~4,3% compared to the baseline)

MAUI ios appBaseThis PRdiff (%)
SOD (Mb)41,9340,5-3,4%
.ipa (Mb)13,4312,85-4,3%

Fixes: #18479

@ivanpovazan

ivanpovazan commented Jul 10, 2023

Copy link
Copy Markdown
MemberAuthor

/cc: @akoeplinger@kotlarmilos@filipnavara

filipnavara
filipnavara reviewed Jul 10, 2023
View reviewed changes
Comment threaddotnet/targets/Xamarin.Shared.Sdk.targets Outdated
</RuntimeHostConfigurationOption>

<!-- Explicitly root the framework assembly due to NativeAOT trimming incompatibility tracked: https://github.com/dotnet/runtime/issues/86649 -->
<TrimmerRootAssembly Include="Microsoft.iOS" />

@filipnavarafilipnavaraJul 10, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
<TrimmerRootAssemblyInclude="Microsoft.iOS" />
<TrimmerRootAssemblyInclude="$(_PlatformAssemblyName)" />

@filipnavarafilipnavaraJul 10, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The name is different for each platform, so this needs to be adjusted. Aside from that, is there a longer description why is this necessary? I suppose with #18519 and #18529 this would no longer be necessary, correct?

@ivanpovazanivanpovazanJul 11, 2023•
edited
Loading

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

Thank you for the correction, it was an oversight. I corrected it.

I suppose with #18519 and #18529 this would no longer be necessary, correct?

If I am not mistaken, there is still an issue with ILC not knowing about [Preserve]. The issue I noted in the comment should track/document all these incompatibilities.

@filipnavarafilipnavaraJul 11, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

If I am not mistaken, there is still an issue with ILC not knowing about [Preserve].

I am almost sure that [Preserve] in the platform assembly is used only for the constructors that would get rooted by the generated code in #18519.

@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@ivanpovazan

ivanpovazan commented Jul 13, 2023

Copy link
Copy Markdown
MemberAuthor

Test failures don't seem to be related.
This is now ready for review.

@ivanpovazan
ivanpovazan marked this pull request as ready for review July 13, 2023 08:31
@ivanpovazanivanpovazan changed the title [DRAFT] NativeAOT: Disable AggressiveAttributeTrimming with ILLinkNativeAOT: Disable AggressiveAttributeTrimming with ILLinkJul 13, 2023
@ivanpovazan
ivanpovazan requested a review from mauroaJuly 13, 2023 08:32
@ivanpovazan

ivanpovazan commented Jul 13, 2023

Copy link
Copy Markdown
MemberAuthor

/azp run

@azure-pipelines

azure-pipelinesBot commented Jul 13, 2023

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@ivanpovazan

ivanpovazan commented Jul 13, 2023

Copy link
Copy Markdown
MemberAuthor

The tests failures do not seem to be related.
All the failures are due to test has timed out at 10s... and the failing tests in this run were not failing in the previous.

@SamMonoRT

SamMonoRT commented Jul 13, 2023

Copy link
Copy Markdown
Member

cc @dalexsoto@mandel-macaque - another one we are targeting for Preview 7

@ivanpovazanivanpovazan mentioned this pull request Jul 13, 2023
Open
52 tasks
@vs-mobiletools-engineering-service2

vs-mobiletools-engineering-service2 commented Jul 13, 2023

Copy link
Copy Markdown
Collaborator

💻 [CI Build] Windows Integration Tests passed 💻

✅ All Windows Integration Tests passed.

Pipeline on Agent
Hash: 9b31fbbfe71e800f989be7fcf25d80180feb8dfd [PR build]

@vs-mobiletools-engineering-service2

vs-mobiletools-engineering-service2 commented Jul 13, 2023

Copy link
Copy Markdown
Collaborator

📚 [PR Build] Artifacts 📚

Packages generated

View packages

Pipeline on Agent
Hash: [PR build]

@vs-mobiletools-engineering-service2

vs-mobiletools-engineering-service2 commented Jul 13, 2023

Copy link
Copy Markdown
Collaborator

💻 [PR Build] Tests on macOS M1 - Mac Big Sur (11.5) passed 💻

✅ All tests on macOS M1 - Mac Big Sur (11.5) passed.

Pipeline on Agent
Hash: [PR build]

@vs-mobiletools-engineering-service2

vs-mobiletools-engineering-service2 commented Jul 13, 2023

Copy link
Copy Markdown
Collaborator

💻 [PR Build] Tests on macOS M1 - Mac Ventura (13.0) passed 💻

✅ All tests on macOS M1 - Mac Ventura (13.0) passed.

Pipeline on Agent
Hash: [PR build]

@vs-mobiletools-engineering-service2

vs-mobiletools-engineering-service2 commented Jul 13, 2023

Copy link
Copy Markdown
Collaborator

✅ API diff for current PR / commit

NET (empty diffs)
  • iOS: (empty diff detected)
  • tvOS: (empty diff detected)
  • MacCatalyst: (empty diff detected)
  • macOS: (empty diff detected)

❗ API diff vs stable (Breaking changes)

.NET (:heavy_exclamation_mark: Breaking changes :heavy_exclamation_mark:)
  • iOS: vsdropsgist (:heavy_exclamation_mark: Breaking changes :heavy_exclamation_mark:)
  • tvOS: vsdropsgist (:heavy_exclamation_mark: Breaking changes :heavy_exclamation_mark:)
  • MacCatalyst: vsdropsgist (:heavy_exclamation_mark: Breaking changes :heavy_exclamation_mark:)
  • macOS: vsdropsgist (:heavy_exclamation_mark: Breaking changes :heavy_exclamation_mark:)
  • Microsoft.iOS vs Microsoft.MacCatalyst: vsdropsgist

ℹ️ Generator diff

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

Pipeline on Agent
Hash: 9b31fbbfe71e800f989be7fcf25d80180feb8dfd [PR build]

@vs-mobiletools-engineering-service2

vs-mobiletools-engineering-service2 commented Jul 13, 2023

Copy link
Copy Markdown
Collaborator

🚀 [CI Build] Test results 🚀

Test results

✅ All tests passed on VSTS: simulator tests.

🎉 All 92 tests passed 🎉

Tests counts

⚠️ bcl: No tests selected. Html Report (VSDrops)Download
✅ cecil: All 1 tests passed. Html Report (VSDrops)Download
✅ dotnettests: All 1 tests passed. Html Report (VSDrops)Download
✅ fsharp: All 4 tests passed. Html Report (VSDrops)Download
✅ framework: All 4 tests passed. Html Report (VSDrops)Download
✅ generator: All 1 tests passed. Html Report (VSDrops)Download
✅ interdependent_binding_projects: All 4 tests passed. Html Report (VSDrops)Download
⚠️ install_source: No tests selected. Html Report (VSDrops)Download
✅ introspection: All 4 tests passed. Html Report (VSDrops)Download
✅ linker: All 40 tests passed. Html Report (VSDrops)Download
⚠️ mac_binding_project: No tests selected. Html Report (VSDrops)Download
⚠️ mmp: No tests selected. Html Report (VSDrops)Download
⚠️ mononative: No tests selected. Html Report (VSDrops)Download
✅ monotouch: All 26 tests passed. Html Report (VSDrops)Download
✅ msbuild: All 2 tests passed. Html Report (VSDrops)Download
⚠️ mtouch: No tests selected. Html Report (VSDrops)Download
⚠️ xammac: No tests selected. Html Report (VSDrops)Download
✅ xcframework: All 4 tests passed. Html Report (VSDrops)Download
✅ xtro: All 1 tests passed. Html Report (VSDrops)Download

Pipeline on Agent
Hash: 9b31fbbfe71e800f989be7fcf25d80180feb8dfd [PR build]

@dalexsoto
dalexsoto merged commit b68efbe into dotnet:net8.0Jul 17, 2023
This was referenced Jun 30, 2023
Closed
Closed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dalexsotodalexsotodalexsoto approved these changes
@rolfbjarnerolfbjarneAwaiting requested review from rolfbjarne
@akoeplingerakoeplingerAwaiting requested review from akoeplinger
@mauroamauroaAwaiting requested review from mauroa
@mandel-macaquemandel-macaqueAwaiting requested review from mandel-macaque
+1 more reviewer
@filipnavarafilipnavarafilipnavara left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@ivanpovazan@vs-mobiletools-engineering-service2@SamMonoRT@dalexsoto@filipnavara
, 'i'); if (__m === '*' || __re.test(location.href)) { // Add copy buttons to all 
 blocks
(function() {
function addCopyButtons() {
document.querySelectorAll('pre code').forEach(function(codeBlock) {
if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;
codeBlock.parentElement.setAttribute('data-copy-added', 'true');
var btn = document.createElement('button');
btn.textContent = 'Copy';
btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';
btn.onmouseover = function() { this.style.opacity = '1'; };
btn.onmouseout = function() { this.style.opacity = '0.7'; };
btn.onclick = function() {
navigator.clipboard.writeText(codeBlock.textContent).then(function() {
btn.textContent = 'Copied!';
setTimeout(function() { btn.textContent = 'Copy'; }, 1500);
});
};
codeBlock.parentElement.style.position = 'relative';
codeBlock.parentElement.appendChild(btn);
});
}
addCopyButtons();
// Re-run on dynamic content
var observer = new MutationObserver(addCopyButtons);
observer.observe(document.body, { childList: true, subtree: true });
})();
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '

Skip to content
NativeAOT: Disable AggressiveAttributeTrimming with ILLink - #18545
Merged
dalexsoto merged 5 commits into
dotnet:net8.0from 
ivanpovazan:naot-trim-enable
Jul 17, 2023
Merged
NativeAOT: Disable AggressiveAttributeTrimming with ILLink#18545
dalexsoto merged 5 commits into
dotnet:net8.0from 
ivanpovazan:naot-trim-enable
Conversation
@ivanpovazan
@ivanpovazanivanpovazan
commented
Jul 10, 2023•

edited

Loading

Copy link


Copy Markdown

Member
In the current setup with NativeAOT, during app build we run both ILLink and ILCompiler in cascade.

When _AggressiveAttributeTrimming feature switch is set to true ILLink removes IsTrimmable attribute from assemblies, which means that when we are also running in TrimMode=partial (which translates to --defaultrooting ILC command-line arguments) ILC trimming is disabled completely.
This PR disables _AggressiveAttributeTrimming in the first pass ie during trimming by ILLink and enables it in the second trimming pass performed by ILCompiler.
Additionally, to workaround ILCompiler incompatibility with Microsoft.iOS (and friends) this platform assembly is explicitly rooted when passed to ILCompiler for trimming (this will be fixed once dotnet/runtime#86649 is resolved).
Estimated savings: This change reduces the size of the application bundle by 0,58Mb (or ~4,3% compared to the baseline)
MAUI ios appBaseThis PRdiff (%)
SOD (Mb)41,9340,5-3,4%
.ipa (Mb)13,4312,85-4,3%
Fixes: #18479
@ivanpovazan
ivanpovazan
commented
Jul 10, 2023

Copy link


Copy Markdown

MemberAuthor
/cc: @akoeplinger@kotlarmilos@filipnavara
filipnavara
filipnavara
reviewed
Jul 10, 2023
View reviewed changes
Comment threaddotnet/targets/Xamarin.Shared.Sdk.targets
Outdated

 </RuntimeHostConfigurationOption>

 <!-- Explicitly root the framework assembly due to NativeAOT trimming incompatibility tracked: https://github.com/dotnet/runtime/issues/86649 -->
 <TrimmerRootAssembly Include="Microsoft.iOS" />
@filipnavarafilipnavaraJul 10, 2023

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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


Suggested change

 <TrimmerRootAssemblyInclude="Microsoft.iOS" />
 <TrimmerRootAssemblyInclude="$(_PlatformAssemblyName)" />
@filipnavarafilipnavaraJul 10, 2023

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

The name is different for each platform, so this needs to be adjusted. Aside from that, is there a longer description why is this necessary? I suppose with #18519 and #18529 this would no longer be necessary, correct?
@ivanpovazanivanpovazanJul 11, 2023•

edited

Loading

Copy link


Copy Markdown

MemberAuthor
Choose a reason for hiding this comment

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

Thank you for the correction, it was an oversight. I corrected it.
I suppose with #18519 and #18529 this would no longer be necessary, correct?
If I am not mistaken, there is still an issue with ILC not knowing about [Preserve]. The issue I noted in the comment should track/document all these incompatibilities.
@filipnavarafilipnavaraJul 11, 2023

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

If I am not mistaken, there is still an issue with ILC not knowing about [Preserve].
I am almost sure that [Preserve] in the platform assembly is used only for the constructors that would get rooted by the generated code in #18519.
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@ivanpovazan
ivanpovazan
commented
Jul 13, 2023

Copy link


Copy Markdown

MemberAuthor
Test failures don't seem to be related.

This is now ready for review.
@ivanpovazan
ivanpovazan
marked this pull request as ready for review
July 13, 2023 08:31
@ivanpovazanivanpovazan
changed the title
[DRAFT] NativeAOT: Disable AggressiveAttributeTrimming with ILLinkNativeAOT: Disable AggressiveAttributeTrimming with ILLinkJul 13, 2023
@ivanpovazan
ivanpovazan
requested a review
from mauroaJuly 13, 2023 08:32
@ivanpovazan
ivanpovazan
commented
Jul 13, 2023

Copy link


Copy Markdown

MemberAuthor
/azp run
@azure-pipelines
azure-pipelinesBot
commented
Jul 13, 2023

Copy link


Copy Markdown


Azure Pipelines successfully started running 1 pipeline(s).
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@ivanpovazan
ivanpovazan
commented
Jul 13, 2023

Copy link


Copy Markdown

MemberAuthor
The tests failures do not seem to be related.

All the failures are due to test has timed out at 10s... and the failing tests in this run were not failing in the previous.
@SamMonoRT
SamMonoRT
commented
Jul 13, 2023

Copy link


Copy Markdown

Member
cc @dalexsoto@mandel-macaque - another one we are targeting for Preview 7
@ivanpovazanivanpovazan
mentioned this pull request
Jul 13, 2023

Open

52 tasks
@vs-mobiletools-engineering-service2
vs-mobiletools-engineering-service2
commented
Jul 13, 2023

Copy link


Copy Markdown

Collaborator
💻 [CI Build] Windows Integration Tests passed 💻
✅ All Windows Integration Tests passed.
Pipeline on Agent

Hash: 9b31fbbfe71e800f989be7fcf25d80180feb8dfd [PR build]
@vs-mobiletools-engineering-service2
vs-mobiletools-engineering-service2
commented
Jul 13, 2023

Copy link


Copy Markdown

Collaborator
📚 [PR Build] Artifacts 📚
Packages generated
View packages
Pipeline on Agent

Hash:  [PR build]
@vs-mobiletools-engineering-service2
vs-mobiletools-engineering-service2
commented
Jul 13, 2023

Copy link


Copy Markdown

Collaborator
💻 [PR Build] Tests on macOS M1 - Mac Big Sur (11.5) passed 💻
✅ All tests on macOS M1 - Mac Big Sur (11.5) passed.
Pipeline on Agent

Hash:  [PR build]
@vs-mobiletools-engineering-service2
vs-mobiletools-engineering-service2
commented
Jul 13, 2023

Copy link


Copy Markdown

Collaborator
💻 [PR Build] Tests on macOS M1 - Mac Ventura (13.0) passed 💻
✅ All tests on macOS M1 - Mac Ventura (13.0) passed.
Pipeline on Agent

Hash:  [PR build]
@vs-mobiletools-engineering-service2
vs-mobiletools-engineering-service2
commented
Jul 13, 2023

Copy link


Copy Markdown

Collaborator
✅ API diff for current PR / commit
NET (empty diffs)
  • iOS: (empty diff detected)
  • tvOS: (empty diff detected)
  • MacCatalyst: (empty diff detected)
  • macOS: (empty diff detected)
❗ API diff vs stable (Breaking changes)
.NET (:heavy_exclamation_mark: Breaking changes :heavy_exclamation_mark:)
  • iOS: vsdropsgist (:heavy_exclamation_mark: Breaking changes :heavy_exclamation_mark:)
  • tvOS: vsdropsgist (:heavy_exclamation_mark: Breaking changes :heavy_exclamation_mark:)
  • MacCatalyst: vsdropsgist (:heavy_exclamation_mark: Breaking changes :heavy_exclamation_mark:)
  • macOS: vsdropsgist (:heavy_exclamation_mark: Breaking changes :heavy_exclamation_mark:)
  • Microsoft.iOS vs Microsoft.MacCatalyst: vsdropsgist
ℹ️ Generator diff
Generator Diff: vsdrops (html) vsdrops (raw diff) gist (raw diff) - Please review changes)
Pipeline on Agent

Hash: 9b31fbbfe71e800f989be7fcf25d80180feb8dfd [PR build]
@vs-mobiletools-engineering-service2
vs-mobiletools-engineering-service2
commented
Jul 13, 2023

Copy link


Copy Markdown

Collaborator
🚀 [CI Build] Test results 🚀
Test results
✅ All tests passed on VSTS: simulator tests.
🎉 All 92 tests passed 🎉
Tests counts
⚠️ bcl: No tests selected. Html Report (VSDrops)Download

✅ cecil: All 1 tests passed. Html Report (VSDrops)Download

✅ dotnettests: All 1 tests passed. Html Report (VSDrops)Download

✅ fsharp: All 4 tests passed. Html Report (VSDrops)Download

✅ framework: All 4 tests passed. Html Report (VSDrops)Download

✅ generator: All 1 tests passed. Html Report (VSDrops)Download

✅ interdependent_binding_projects: All 4 tests passed. Html Report (VSDrops)Download
⚠️ install_source: No tests selected. Html Report (VSDrops)Download

✅ introspection: All 4 tests passed. Html Report (VSDrops)Download

✅ linker: All 40 tests passed. Html Report (VSDrops)Download
⚠️ mac_binding_project: No tests selected. Html Report (VSDrops)Download
⚠️ mmp: No tests selected. Html Report (VSDrops)Download
⚠️ mononative: No tests selected. Html Report (VSDrops)Download

✅ monotouch: All 26 tests passed. Html Report (VSDrops)Download

✅ msbuild: All 2 tests passed. Html Report (VSDrops)Download
⚠️ mtouch: No tests selected. Html Report (VSDrops)Download
⚠️ xammac: No tests selected. Html Report (VSDrops)Download

✅ xcframework: All 4 tests passed. Html Report (VSDrops)Download

✅ xtro: All 1 tests passed. Html Report (VSDrops)Download
Pipeline on Agent

Hash: 9b31fbbfe71e800f989be7fcf25d80180feb8dfd [PR build]
@dalexsoto
dalexsoto
merged commit b68efbe
into
dotnet:net8.0Jul 17, 2023

This was referenced Jun 30, 2023

Closed


Closed

Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers

@dalexsotodalexsotodalexsoto approved these changes
@rolfbjarnerolfbjarneAwaiting requested review from rolfbjarne
@akoeplingerakoeplingerAwaiting requested review from akoeplinger
@mauroamauroaAwaiting requested review from mauroa
@mandel-macaquemandel-macaqueAwaiting requested review from mandel-macaque

+1 more reviewer

@filipnavarafilipnavarafilipnavara left review comments
Reviewers whose approvals may not affect merge requirements

Assignees


No one assigned


Labels


None yet


Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

5 participants

@ivanpovazan@vs-mobiletools-engineering-service2@SamMonoRT@dalexsoto@filipnavara

, 'i');
if (__m === '*' || __re.test(location.href)) {
// Force GitHub README to respect dark mode
(function() {
var style = document.createElement('style');
style.textContent = '
.markdown-body {
color-scheme: dark light;
}
.markdown-body pre { background: #161b22 !important; }
.markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }
.markdown-body table th, .markdown-body table td { border-color: #30363d !important; }
.markdown-body img { background: #0d1117; }
.markdown-body blockquote { border-left-color: #8b949e; }
.markdown-body hr { border-color: #30363d; }
';
document.head.appendChild(style);
})();
}
} catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
NativeAOT: Disable AggressiveAttributeTrimming with ILLink - #18545
Merged
dalexsoto merged 5 commits into
dotnet:net8.0from 
ivanpovazan:naot-trim-enable
Jul 17, 2023
Merged
NativeAOT: Disable AggressiveAttributeTrimming with ILLink#18545
dalexsoto merged 5 commits into
dotnet:net8.0from 
ivanpovazan:naot-trim-enable
Conversation
@ivanpovazan
@ivanpovazanivanpovazan
commented
Jul 10, 2023•

edited

Loading

Copy link


Copy Markdown

Member
In the current setup with NativeAOT, during app build we run both ILLink and ILCompiler in cascade.

When _AggressiveAttributeTrimming feature switch is set to true ILLink removes IsTrimmable attribute from assemblies, which means that when we are also running in TrimMode=partial (which translates to --defaultrooting ILC command-line arguments) ILC trimming is disabled completely.
This PR disables _AggressiveAttributeTrimming in the first pass ie during trimming by ILLink and enables it in the second trimming pass performed by ILCompiler.
Additionally, to workaround ILCompiler incompatibility with Microsoft.iOS (and friends) this platform assembly is explicitly rooted when passed to ILCompiler for trimming (this will be fixed once dotnet/runtime#86649 is resolved).
Estimated savings: This change reduces the size of the application bundle by 0,58Mb (or ~4,3% compared to the baseline)
MAUI ios appBaseThis PRdiff (%)
SOD (Mb)41,9340,5-3,4%
.ipa (Mb)13,4312,85-4,3%
Fixes: #18479
@ivanpovazan
ivanpovazan
commented
Jul 10, 2023

Copy link


Copy Markdown

MemberAuthor
/cc: @akoeplinger@kotlarmilos@filipnavara
filipnavara
filipnavara
reviewed
Jul 10, 2023
View reviewed changes
Comment threaddotnet/targets/Xamarin.Shared.Sdk.targets
Outdated

 </RuntimeHostConfigurationOption>

 <!-- Explicitly root the framework assembly due to NativeAOT trimming incompatibility tracked: https://github.com/dotnet/runtime/issues/86649 -->
 <TrimmerRootAssembly Include="Microsoft.iOS" />
@filipnavarafilipnavaraJul 10, 2023

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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


Suggested change

 <TrimmerRootAssemblyInclude="Microsoft.iOS" />
 <TrimmerRootAssemblyInclude="$(_PlatformAssemblyName)" />
@filipnavarafilipnavaraJul 10, 2023

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

The name is different for each platform, so this needs to be adjusted. Aside from that, is there a longer description why is this necessary? I suppose with #18519 and #18529 this would no longer be necessary, correct?
@ivanpovazanivanpovazanJul 11, 2023•

edited

Loading

Copy link


Copy Markdown

MemberAuthor
Choose a reason for hiding this comment

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

Thank you for the correction, it was an oversight. I corrected it.
I suppose with #18519 and #18529 this would no longer be necessary, correct?
If I am not mistaken, there is still an issue with ILC not knowing about [Preserve]. The issue I noted in the comment should track/document all these incompatibilities.
@filipnavarafilipnavaraJul 11, 2023

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

If I am not mistaken, there is still an issue with ILC not knowing about [Preserve].
I am almost sure that [Preserve] in the platform assembly is used only for the constructors that would get rooted by the generated code in #18519.
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@ivanpovazan
ivanpovazan
commented
Jul 13, 2023

Copy link


Copy Markdown

MemberAuthor
Test failures don't seem to be related.

This is now ready for review.
@ivanpovazan
ivanpovazan
marked this pull request as ready for review
July 13, 2023 08:31
@ivanpovazanivanpovazan
changed the title
[DRAFT] NativeAOT: Disable AggressiveAttributeTrimming with ILLinkNativeAOT: Disable AggressiveAttributeTrimming with ILLinkJul 13, 2023
@ivanpovazan
ivanpovazan
requested a review
from mauroaJuly 13, 2023 08:32
@ivanpovazan
ivanpovazan
commented
Jul 13, 2023

Copy link


Copy Markdown

MemberAuthor
/azp run
@azure-pipelines
azure-pipelinesBot
commented
Jul 13, 2023

Copy link


Copy Markdown


Azure Pipelines successfully started running 1 pipeline(s).
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@ivanpovazan
ivanpovazan
commented
Jul 13, 2023

Copy link


Copy Markdown

MemberAuthor
The tests failures do not seem to be related.

All the failures are due to test has timed out at 10s... and the failing tests in this run were not failing in the previous.
@SamMonoRT
SamMonoRT
commented
Jul 13, 2023

Copy link


Copy Markdown

Member
cc @dalexsoto@mandel-macaque - another one we are targeting for Preview 7
@ivanpovazanivanpovazan
mentioned this pull request
Jul 13, 2023

Open

52 tasks
@vs-mobiletools-engineering-service2
vs-mobiletools-engineering-service2
commented
Jul 13, 2023

Copy link


Copy Markdown

Collaborator
💻 [CI Build] Windows Integration Tests passed 💻
✅ All Windows Integration Tests passed.
Pipeline on Agent

Hash: 9b31fbbfe71e800f989be7fcf25d80180feb8dfd [PR build]
@vs-mobiletools-engineering-service2
vs-mobiletools-engineering-service2
commented
Jul 13, 2023

Copy link


Copy Markdown

Collaborator
📚 [PR Build] Artifacts 📚
Packages generated
View packages
Pipeline on Agent

Hash:  [PR build]
@vs-mobiletools-engineering-service2
vs-mobiletools-engineering-service2
commented
Jul 13, 2023

Copy link


Copy Markdown

Collaborator
💻 [PR Build] Tests on macOS M1 - Mac Big Sur (11.5) passed 💻
✅ All tests on macOS M1 - Mac Big Sur (11.5) passed.
Pipeline on Agent

Hash:  [PR build]
@vs-mobiletools-engineering-service2
vs-mobiletools-engineering-service2
commented
Jul 13, 2023

Copy link


Copy Markdown

Collaborator
💻 [PR Build] Tests on macOS M1 - Mac Ventura (13.0) passed 💻
✅ All tests on macOS M1 - Mac Ventura (13.0) passed.
Pipeline on Agent

Hash:  [PR build]
@vs-mobiletools-engineering-service2
vs-mobiletools-engineering-service2
commented
Jul 13, 2023

Copy link


Copy Markdown

Collaborator
✅ API diff for current PR / commit
NET (empty diffs)
  • iOS: (empty diff detected)
  • tvOS: (empty diff detected)
  • MacCatalyst: (empty diff detected)
  • macOS: (empty diff detected)
❗ API diff vs stable (Breaking changes)
.NET (:heavy_exclamation_mark: Breaking changes :heavy_exclamation_mark:)
  • iOS: vsdropsgist (:heavy_exclamation_mark: Breaking changes :heavy_exclamation_mark:)
  • tvOS: vsdropsgist (:heavy_exclamation_mark: Breaking changes :heavy_exclamation_mark:)
  • MacCatalyst: vsdropsgist (:heavy_exclamation_mark: Breaking changes :heavy_exclamation_mark:)
  • macOS: vsdropsgist (:heavy_exclamation_mark: Breaking changes :heavy_exclamation_mark:)
  • Microsoft.iOS vs Microsoft.MacCatalyst: vsdropsgist
ℹ️ Generator diff
Generator Diff: vsdrops (html) vsdrops (raw diff) gist (raw diff) - Please review changes)
Pipeline on Agent

Hash: 9b31fbbfe71e800f989be7fcf25d80180feb8dfd [PR build]
@vs-mobiletools-engineering-service2
vs-mobiletools-engineering-service2
commented
Jul 13, 2023

Copy link


Copy Markdown

Collaborator
🚀 [CI Build] Test results 🚀
Test results
✅ All tests passed on VSTS: simulator tests.
🎉 All 92 tests passed 🎉
Tests counts
⚠️ bcl: No tests selected. Html Report (VSDrops)Download

✅ cecil: All 1 tests passed. Html Report (VSDrops)Download

✅ dotnettests: All 1 tests passed. Html Report (VSDrops)Download

✅ fsharp: All 4 tests passed. Html Report (VSDrops)Download

✅ framework: All 4 tests passed. Html Report (VSDrops)Download

✅ generator: All 1 tests passed. Html Report (VSDrops)Download

✅ interdependent_binding_projects: All 4 tests passed. Html Report (VSDrops)Download
⚠️ install_source: No tests selected. Html Report (VSDrops)Download

✅ introspection: All 4 tests passed. Html Report (VSDrops)Download

✅ linker: All 40 tests passed. Html Report (VSDrops)Download
⚠️ mac_binding_project: No tests selected. Html Report (VSDrops)Download
⚠️ mmp: No tests selected. Html Report (VSDrops)Download
⚠️ mononative: No tests selected. Html Report (VSDrops)Download

✅ monotouch: All 26 tests passed. Html Report (VSDrops)Download

✅ msbuild: All 2 tests passed. Html Report (VSDrops)Download
⚠️ mtouch: No tests selected. Html Report (VSDrops)Download
⚠️ xammac: No tests selected. Html Report (VSDrops)Download

✅ xcframework: All 4 tests passed. Html Report (VSDrops)Download

✅ xtro: All 1 tests passed. Html Report (VSDrops)Download
Pipeline on Agent

Hash: 9b31fbbfe71e800f989be7fcf25d80180feb8dfd [PR build]
@dalexsoto
dalexsoto
merged commit b68efbe
into
dotnet:net8.0Jul 17, 2023

This was referenced Jun 30, 2023

Closed


Closed

Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers

@dalexsotodalexsotodalexsoto approved these changes
@rolfbjarnerolfbjarneAwaiting requested review from rolfbjarne
@akoeplingerakoeplingerAwaiting requested review from akoeplinger
@mauroamauroaAwaiting requested review from mauroa
@mandel-macaquemandel-macaqueAwaiting requested review from mandel-macaque

+1 more reviewer

@filipnavarafilipnavarafilipnavara left review comments
Reviewers whose approvals may not affect merge requirements

Assignees


No one assigned


Labels


None yet


Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

5 participants

@ivanpovazan@vs-mobiletools-engineering-service2@SamMonoRT@dalexsoto@filipnavara

, 'i');
if (__m === '*' || __re.test(location.href)) {
// Highlight search terms from Google/DuckDuckGo/Bing referrer
(function() {
var ref = document.referrer;
var terms = [];
if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {
var url = new URL(ref);
var q = url.searchParams.get('q') || url.searchParams.get('p');
if (q) {
terms = q.split(/\s+/).filter(function(t) { return t.length > 2; });
}
}
if (terms.length === 0) return;
var style = document.createElement('style');
style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';
document.head.appendChild(style);
function highlight(node) {
if (node.nodeType === 3) { // text node
var text = node.textContent;
var found = false;
terms.forEach(function(term) {
var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\]\\]/g, '\\') + ')', 'gi');
if (regex.test(text)) {
found = true;
var frag = document.createDocumentFragment();
var parts = text.split(regex);
parts.forEach(function(part, i) {
if (i % 2 === 0) {
frag.appendChild(document.createTextNode(part));
} else {
var span = document.createElement('span');
span.className = 'userscript-highlight';
span.textContent = part;
frag.appendChild(span);
}
});
node.parentNode.replaceChild(frag, node);
}
});
} else if (node.nodeType === 1 && node.childNodes) { // element
var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];
if (!skipTags.includes(node.tagName)) {
Array.from(node.childNodes).forEach(highlight);
}
}
}
highlight(document.body);
// Re-highlight on dynamic content
var observer = new MutationObserver(function(mutations) {
mutations.forEach(function(m) {
m.addedNodes.forEach(function(node) {
if (node.nodeType === 1 || node.nodeType === 3) highlight(node);
});
});
});
observer.observe(document.body, { childList: true, subtree: true });
})();
}
} catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
NativeAOT: Disable AggressiveAttributeTrimming with ILLink - #18545
Merged
dalexsoto merged 5 commits into
dotnet:net8.0from 
ivanpovazan:naot-trim-enable
Jul 17, 2023
Merged
NativeAOT: Disable AggressiveAttributeTrimming with ILLink#18545
dalexsoto merged 5 commits into
dotnet:net8.0from 
ivanpovazan:naot-trim-enable
Conversation
@ivanpovazan
@ivanpovazanivanpovazan
commented
Jul 10, 2023•

edited

Loading

Copy link


Copy Markdown

Member
In the current setup with NativeAOT, during app build we run both ILLink and ILCompiler in cascade.

When _AggressiveAttributeTrimming feature switch is set to true ILLink removes IsTrimmable attribute from assemblies, which means that when we are also running in TrimMode=partial (which translates to --defaultrooting ILC command-line arguments) ILC trimming is disabled completely.
This PR disables _AggressiveAttributeTrimming in the first pass ie during trimming by ILLink and enables it in the second trimming pass performed by ILCompiler.
Additionally, to workaround ILCompiler incompatibility with Microsoft.iOS (and friends) this platform assembly is explicitly rooted when passed to ILCompiler for trimming (this will be fixed once dotnet/runtime#86649 is resolved).
Estimated savings: This change reduces the size of the application bundle by 0,58Mb (or ~4,3% compared to the baseline)
MAUI ios appBaseThis PRdiff (%)
SOD (Mb)41,9340,5-3,4%
.ipa (Mb)13,4312,85-4,3%
Fixes: #18479
@ivanpovazan
ivanpovazan
commented
Jul 10, 2023

Copy link


Copy Markdown

MemberAuthor
/cc: @akoeplinger@kotlarmilos@filipnavara
filipnavara
filipnavara
reviewed
Jul 10, 2023
View reviewed changes
Comment threaddotnet/targets/Xamarin.Shared.Sdk.targets
Outdated

 </RuntimeHostConfigurationOption>

 <!-- Explicitly root the framework assembly due to NativeAOT trimming incompatibility tracked: https://github.com/dotnet/runtime/issues/86649 -->
 <TrimmerRootAssembly Include="Microsoft.iOS" />
@filipnavarafilipnavaraJul 10, 2023

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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


Suggested change

 <TrimmerRootAssemblyInclude="Microsoft.iOS" />
 <TrimmerRootAssemblyInclude="$(_PlatformAssemblyName)" />
@filipnavarafilipnavaraJul 10, 2023

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

The name is different for each platform, so this needs to be adjusted. Aside from that, is there a longer description why is this necessary? I suppose with #18519 and #18529 this would no longer be necessary, correct?
@ivanpovazanivanpovazanJul 11, 2023•

edited

Loading

Copy link


Copy Markdown

MemberAuthor
Choose a reason for hiding this comment

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

Thank you for the correction, it was an oversight. I corrected it.
I suppose with #18519 and #18529 this would no longer be necessary, correct?
If I am not mistaken, there is still an issue with ILC not knowing about [Preserve]. The issue I noted in the comment should track/document all these incompatibilities.
@filipnavarafilipnavaraJul 11, 2023

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

If I am not mistaken, there is still an issue with ILC not knowing about [Preserve].
I am almost sure that [Preserve] in the platform assembly is used only for the constructors that would get rooted by the generated code in #18519.
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@ivanpovazan
ivanpovazan
commented
Jul 13, 2023

Copy link


Copy Markdown

MemberAuthor
Test failures don't seem to be related.

This is now ready for review.
@ivanpovazan
ivanpovazan
marked this pull request as ready for review
July 13, 2023 08:31
@ivanpovazanivanpovazan
changed the title
[DRAFT] NativeAOT: Disable AggressiveAttributeTrimming with ILLinkNativeAOT: Disable AggressiveAttributeTrimming with ILLinkJul 13, 2023
@ivanpovazan
ivanpovazan
requested a review
from mauroaJuly 13, 2023 08:32
@ivanpovazan
ivanpovazan
commented
Jul 13, 2023

Copy link


Copy Markdown

MemberAuthor
/azp run
@azure-pipelines
azure-pipelinesBot
commented
Jul 13, 2023

Copy link


Copy Markdown


Azure Pipelines successfully started running 1 pipeline(s).
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@ivanpovazan
ivanpovazan
commented
Jul 13, 2023

Copy link


Copy Markdown

MemberAuthor
The tests failures do not seem to be related.

All the failures are due to test has timed out at 10s... and the failing tests in this run were not failing in the previous.
@SamMonoRT
SamMonoRT
commented
Jul 13, 2023

Copy link


Copy Markdown

Member
cc @dalexsoto@mandel-macaque - another one we are targeting for Preview 7
@ivanpovazanivanpovazan
mentioned this pull request
Jul 13, 2023

Open

52 tasks
@vs-mobiletools-engineering-service2
vs-mobiletools-engineering-service2
commented
Jul 13, 2023

Copy link


Copy Markdown

Collaborator
💻 [CI Build] Windows Integration Tests passed 💻
✅ All Windows Integration Tests passed.
Pipeline on Agent

Hash: 9b31fbbfe71e800f989be7fcf25d80180feb8dfd [PR build]
@vs-mobiletools-engineering-service2
vs-mobiletools-engineering-service2
commented
Jul 13, 2023

Copy link


Copy Markdown

Collaborator
📚 [PR Build] Artifacts 📚
Packages generated
View packages
Pipeline on Agent

Hash:  [PR build]
@vs-mobiletools-engineering-service2
vs-mobiletools-engineering-service2
commented
Jul 13, 2023

Copy link


Copy Markdown

Collaborator
💻 [PR Build] Tests on macOS M1 - Mac Big Sur (11.5) passed 💻
✅ All tests on macOS M1 - Mac Big Sur (11.5) passed.
Pipeline on Agent

Hash:  [PR build]
@vs-mobiletools-engineering-service2
vs-mobiletools-engineering-service2
commented
Jul 13, 2023

Copy link


Copy Markdown

Collaborator
💻 [PR Build] Tests on macOS M1 - Mac Ventura (13.0) passed 💻
✅ All tests on macOS M1 - Mac Ventura (13.0) passed.
Pipeline on Agent

Hash:  [PR build]
@vs-mobiletools-engineering-service2
vs-mobiletools-engineering-service2
commented
Jul 13, 2023

Copy link


Copy Markdown

Collaborator
✅ API diff for current PR / commit
NET (empty diffs)
  • iOS: (empty diff detected)
  • tvOS: (empty diff detected)
  • MacCatalyst: (empty diff detected)
  • macOS: (empty diff detected)
❗ API diff vs stable (Breaking changes)
.NET (:heavy_exclamation_mark: Breaking changes :heavy_exclamation_mark:)
  • iOS: vsdropsgist (:heavy_exclamation_mark: Breaking changes :heavy_exclamation_mark:)
  • tvOS: vsdropsgist (:heavy_exclamation_mark: Breaking changes :heavy_exclamation_mark:)
  • MacCatalyst: vsdropsgist (:heavy_exclamation_mark: Breaking changes :heavy_exclamation_mark:)
  • macOS: vsdropsgist (:heavy_exclamation_mark: Breaking changes :heavy_exclamation_mark:)
  • Microsoft.iOS vs Microsoft.MacCatalyst: vsdropsgist
ℹ️ Generator diff
Generator Diff: vsdrops (html) vsdrops (raw diff) gist (raw diff) - Please review changes)
Pipeline on Agent

Hash: 9b31fbbfe71e800f989be7fcf25d80180feb8dfd [PR build]
@vs-mobiletools-engineering-service2
vs-mobiletools-engineering-service2
commented
Jul 13, 2023

Copy link


Copy Markdown

Collaborator
🚀 [CI Build] Test results 🚀
Test results
✅ All tests passed on VSTS: simulator tests.
🎉 All 92 tests passed 🎉
Tests counts
⚠️ bcl: No tests selected. Html Report (VSDrops)Download

✅ cecil: All 1 tests passed. Html Report (VSDrops)Download

✅ dotnettests: All 1 tests passed. Html Report (VSDrops)Download

✅ fsharp: All 4 tests passed. Html Report (VSDrops)Download

✅ framework: All 4 tests passed. Html Report (VSDrops)Download

✅ generator: All 1 tests passed. Html Report (VSDrops)Download

✅ interdependent_binding_projects: All 4 tests passed. Html Report (VSDrops)Download
⚠️ install_source: No tests selected. Html Report (VSDrops)Download

✅ introspection: All 4 tests passed. Html Report (VSDrops)Download

✅ linker: All 40 tests passed. Html Report (VSDrops)Download
⚠️ mac_binding_project: No tests selected. Html Report (VSDrops)Download
⚠️ mmp: No tests selected. Html Report (VSDrops)Download
⚠️ mononative: No tests selected. Html Report (VSDrops)Download

✅ monotouch: All 26 tests passed. Html Report (VSDrops)Download

✅ msbuild: All 2 tests passed. Html Report (VSDrops)Download
⚠️ mtouch: No tests selected. Html Report (VSDrops)Download
⚠️ xammac: No tests selected. Html Report (VSDrops)Download

✅ xcframework: All 4 tests passed. Html Report (VSDrops)Download

✅ xtro: All 1 tests passed. Html Report (VSDrops)Download
Pipeline on Agent

Hash: 9b31fbbfe71e800f989be7fcf25d80180feb8dfd [PR build]
@dalexsoto
dalexsoto
merged commit b68efbe
into
dotnet:net8.0Jul 17, 2023

This was referenced Jun 30, 2023

Closed


Closed

Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers

@dalexsotodalexsotodalexsoto approved these changes
@rolfbjarnerolfbjarneAwaiting requested review from rolfbjarne
@akoeplingerakoeplingerAwaiting requested review from akoeplinger
@mauroamauroaAwaiting requested review from mauroa
@mandel-macaquemandel-macaqueAwaiting requested review from mandel-macaque

+1 more reviewer

@filipnavarafilipnavarafilipnavara left review comments
Reviewers whose approvals may not affect merge requirements

Assignees


No one assigned


Labels


None yet


Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

5 participants

@ivanpovazan@vs-mobiletools-engineering-service2@SamMonoRT@dalexsoto@filipnavara

, 'i');
if (__m === '*' || __re.test(location.href)) {
// Strip utm_, fbclid, gclid, etc. from all links on page
(function() {
var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',
'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',
'ref', 'ref_src', 'source', 'medium', 'campaign'];
function cleanUrl(url) {
try {
var u = new URL(url, window.location.origin);
var changed = false;
trackingParams.forEach(function(p) {
if (u.searchParams.has(p)) {
u.searchParams.delete(p);
changed = true;
}
});
return changed ? u.toString() : url;
} catch (e) {
return url;
}
}
function cleanLinks() {
document.querySelectorAll('a[href]').forEach(function(a) {
var clean = cleanUrl(a.href);
if (clean !== a.href) a.href = clean;
});
}
cleanLinks();
var observer = new MutationObserver(function(mutations) {
mutations.forEach(function(m) {
m.addedNodes.forEach(function(node) {
if (node.nodeType === 1) {
if (node.tagName === 'A') cleanLinks();
node.querySelectorAll('a[href]').forEach(function(a) {
var clean = cleanUrl(a.href);
if (clean !== a.href) a.href = clean;
});
}
});
});
});
observer.observe(document.body, { childList: true, subtree: true });
})();
}
} catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); }
})();
(function(){
try {
var __m = "youtube.com";
var __re = new RegExp('^' + "youtube\\.com" + '

Skip to content
NativeAOT: Disable AggressiveAttributeTrimming with ILLink - #18545
Merged
dalexsoto merged 5 commits into
dotnet:net8.0from 
ivanpovazan:naot-trim-enable
Jul 17, 2023
Merged
NativeAOT: Disable AggressiveAttributeTrimming with ILLink#18545
dalexsoto merged 5 commits into
dotnet:net8.0from 
ivanpovazan:naot-trim-enable
Conversation
@ivanpovazan
@ivanpovazanivanpovazan
commented
Jul 10, 2023•

edited

Loading

Copy link


Copy Markdown

Member
In the current setup with NativeAOT, during app build we run both ILLink and ILCompiler in cascade.

When _AggressiveAttributeTrimming feature switch is set to true ILLink removes IsTrimmable attribute from assemblies, which means that when we are also running in TrimMode=partial (which translates to --defaultrooting ILC command-line arguments) ILC trimming is disabled completely.
This PR disables _AggressiveAttributeTrimming in the first pass ie during trimming by ILLink and enables it in the second trimming pass performed by ILCompiler.
Additionally, to workaround ILCompiler incompatibility with Microsoft.iOS (and friends) this platform assembly is explicitly rooted when passed to ILCompiler for trimming (this will be fixed once dotnet/runtime#86649 is resolved).
Estimated savings: This change reduces the size of the application bundle by 0,58Mb (or ~4,3% compared to the baseline)
MAUI ios appBaseThis PRdiff (%)
SOD (Mb)41,9340,5-3,4%
.ipa (Mb)13,4312,85-4,3%
Fixes: #18479
@ivanpovazan
ivanpovazan
commented
Jul 10, 2023

Copy link


Copy Markdown

MemberAuthor
/cc: @akoeplinger@kotlarmilos@filipnavara
filipnavara
filipnavara
reviewed
Jul 10, 2023
View reviewed changes
Comment threaddotnet/targets/Xamarin.Shared.Sdk.targets
Outdated

 </RuntimeHostConfigurationOption>

 <!-- Explicitly root the framework assembly due to NativeAOT trimming incompatibility tracked: https://github.com/dotnet/runtime/issues/86649 -->
 <TrimmerRootAssembly Include="Microsoft.iOS" />
@filipnavarafilipnavaraJul 10, 2023

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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


Suggested change

 <TrimmerRootAssemblyInclude="Microsoft.iOS" />
 <TrimmerRootAssemblyInclude="$(_PlatformAssemblyName)" />
@filipnavarafilipnavaraJul 10, 2023

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

The name is different for each platform, so this needs to be adjusted. Aside from that, is there a longer description why is this necessary? I suppose with #18519 and #18529 this would no longer be necessary, correct?
@ivanpovazanivanpovazanJul 11, 2023•

edited

Loading

Copy link


Copy Markdown

MemberAuthor
Choose a reason for hiding this comment

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

Thank you for the correction, it was an oversight. I corrected it.
I suppose with #18519 and #18529 this would no longer be necessary, correct?
If I am not mistaken, there is still an issue with ILC not knowing about [Preserve]. The issue I noted in the comment should track/document all these incompatibilities.
@filipnavarafilipnavaraJul 11, 2023

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

If I am not mistaken, there is still an issue with ILC not knowing about [Preserve].
I am almost sure that [Preserve] in the platform assembly is used only for the constructors that would get rooted by the generated code in #18519.
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@ivanpovazan
ivanpovazan
commented
Jul 13, 2023

Copy link


Copy Markdown

MemberAuthor
Test failures don't seem to be related.

This is now ready for review.
@ivanpovazan
ivanpovazan
marked this pull request as ready for review
July 13, 2023 08:31
@ivanpovazanivanpovazan
changed the title
[DRAFT] NativeAOT: Disable AggressiveAttributeTrimming with ILLinkNativeAOT: Disable AggressiveAttributeTrimming with ILLinkJul 13, 2023
@ivanpovazan
ivanpovazan
requested a review
from mauroaJuly 13, 2023 08:32
@ivanpovazan
ivanpovazan
commented
Jul 13, 2023

Copy link


Copy Markdown

MemberAuthor
/azp run
@azure-pipelines
azure-pipelinesBot
commented
Jul 13, 2023

Copy link


Copy Markdown


Azure Pipelines successfully started running 1 pipeline(s).
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@ivanpovazan
ivanpovazan
commented
Jul 13, 2023

Copy link


Copy Markdown

MemberAuthor
The tests failures do not seem to be related.

All the failures are due to test has timed out at 10s... and the failing tests in this run were not failing in the previous.
@SamMonoRT
SamMonoRT
commented
Jul 13, 2023

Copy link


Copy Markdown

Member
cc @dalexsoto@mandel-macaque - another one we are targeting for Preview 7
@ivanpovazanivanpovazan
mentioned this pull request
Jul 13, 2023

Open

52 tasks
@vs-mobiletools-engineering-service2
vs-mobiletools-engineering-service2
commented
Jul 13, 2023

Copy link


Copy Markdown

Collaborator
💻 [CI Build] Windows Integration Tests passed 💻
✅ All Windows Integration Tests passed.
Pipeline on Agent

Hash: 9b31fbbfe71e800f989be7fcf25d80180feb8dfd [PR build]
@vs-mobiletools-engineering-service2
vs-mobiletools-engineering-service2
commented
Jul 13, 2023

Copy link


Copy Markdown

Collaborator
📚 [PR Build] Artifacts 📚
Packages generated
View packages
Pipeline on Agent

Hash:  [PR build]
@vs-mobiletools-engineering-service2
vs-mobiletools-engineering-service2
commented
Jul 13, 2023

Copy link


Copy Markdown

Collaborator
💻 [PR Build] Tests on macOS M1 - Mac Big Sur (11.5) passed 💻
✅ All tests on macOS M1 - Mac Big Sur (11.5) passed.
Pipeline on Agent

Hash:  [PR build]
@vs-mobiletools-engineering-service2
vs-mobiletools-engineering-service2
commented
Jul 13, 2023

Copy link


Copy Markdown

Collaborator
💻 [PR Build] Tests on macOS M1 - Mac Ventura (13.0) passed 💻
✅ All tests on macOS M1 - Mac Ventura (13.0) passed.
Pipeline on Agent

Hash:  [PR build]
@vs-mobiletools-engineering-service2
vs-mobiletools-engineering-service2
commented
Jul 13, 2023

Copy link


Copy Markdown

Collaborator
✅ API diff for current PR / commit
NET (empty diffs)
  • iOS: (empty diff detected)
  • tvOS: (empty diff detected)
  • MacCatalyst: (empty diff detected)
  • macOS: (empty diff detected)
❗ API diff vs stable (Breaking changes)
.NET (:heavy_exclamation_mark: Breaking changes :heavy_exclamation_mark:)
  • iOS: vsdropsgist (:heavy_exclamation_mark: Breaking changes :heavy_exclamation_mark:)
  • tvOS: vsdropsgist (:heavy_exclamation_mark: Breaking changes :heavy_exclamation_mark:)
  • MacCatalyst: vsdropsgist (:heavy_exclamation_mark: Breaking changes :heavy_exclamation_mark:)
  • macOS: vsdropsgist (:heavy_exclamation_mark: Breaking changes :heavy_exclamation_mark:)
  • Microsoft.iOS vs Microsoft.MacCatalyst: vsdropsgist
ℹ️ Generator diff
Generator Diff: vsdrops (html) vsdrops (raw diff) gist (raw diff) - Please review changes)
Pipeline on Agent

Hash: 9b31fbbfe71e800f989be7fcf25d80180feb8dfd [PR build]
@vs-mobiletools-engineering-service2
vs-mobiletools-engineering-service2
commented
Jul 13, 2023

Copy link


Copy Markdown

Collaborator
🚀 [CI Build] Test results 🚀
Test results
✅ All tests passed on VSTS: simulator tests.
🎉 All 92 tests passed 🎉
Tests counts
⚠️ bcl: No tests selected. Html Report (VSDrops)Download

✅ cecil: All 1 tests passed. Html Report (VSDrops)Download

✅ dotnettests: All 1 tests passed. Html Report (VSDrops)Download

✅ fsharp: All 4 tests passed. Html Report (VSDrops)Download

✅ framework: All 4 tests passed. Html Report (VSDrops)Download

✅ generator: All 1 tests passed. Html Report (VSDrops)Download

✅ interdependent_binding_projects: All 4 tests passed. Html Report (VSDrops)Download
⚠️ install_source: No tests selected. Html Report (VSDrops)Download

✅ introspection: All 4 tests passed. Html Report (VSDrops)Download

✅ linker: All 40 tests passed. Html Report (VSDrops)Download
⚠️ mac_binding_project: No tests selected. Html Report (VSDrops)Download
⚠️ mmp: No tests selected. Html Report (VSDrops)Download
⚠️ mononative: No tests selected. Html Report (VSDrops)Download

✅ monotouch: All 26 tests passed. Html Report (VSDrops)Download

✅ msbuild: All 2 tests passed. Html Report (VSDrops)Download
⚠️ mtouch: No tests selected. Html Report (VSDrops)Download
⚠️ xammac: No tests selected. Html Report (VSDrops)Download

✅ xcframework: All 4 tests passed. Html Report (VSDrops)Download

✅ xtro: All 1 tests passed. Html Report (VSDrops)Download
Pipeline on Agent

Hash: 9b31fbbfe71e800f989be7fcf25d80180feb8dfd [PR build]
@dalexsoto
dalexsoto
merged commit b68efbe
into
dotnet:net8.0Jul 17, 2023

This was referenced Jun 30, 2023

Closed


Closed

Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers

@dalexsotodalexsotodalexsoto approved these changes
@rolfbjarnerolfbjarneAwaiting requested review from rolfbjarne
@akoeplingerakoeplingerAwaiting requested review from akoeplinger
@mauroamauroaAwaiting requested review from mauroa
@mandel-macaquemandel-macaqueAwaiting requested review from mandel-macaque

+1 more reviewer

@filipnavarafilipnavarafilipnavara left review comments
Reviewers whose approvals may not affect merge requirements

Assignees


No one assigned


Labels


None yet


Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

5 participants

@ivanpovazan@vs-mobiletools-engineering-service2@SamMonoRT@dalexsoto@filipnavara

, 'i');
if (__m === '*' || __re.test(location.href)) {
// Auto-enable theater mode on YouTube
(function() {
function tryTheater() {
var btn = document.querySelector('button[aria-label="Theater mode"], ytd-player #player button[title="Theater mode"]');
if (btn && !btn.classList.contains('activated')) {
btn.click();
}
}
// Try immediately
tryTheater();
// Try after navigation (SPA)
var lastUrl = location.href;
setInterval(function() {
if (location.href !== lastUrl) {
lastUrl = location.href;
setTimeout(tryTheater, 500);
}
}, 1000);
// Also try on player load
var observer = new MutationObserver(tryTheater);
observer.observe(document.body, { childList: true, subtree: true });
})();
}
} catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
NativeAOT: Disable AggressiveAttributeTrimming with ILLink - #18545
Merged
dalexsoto merged 5 commits into
dotnet:net8.0from 
ivanpovazan:naot-trim-enable
Jul 17, 2023
Merged
NativeAOT: Disable AggressiveAttributeTrimming with ILLink#18545
dalexsoto merged 5 commits into
dotnet:net8.0from 
ivanpovazan:naot-trim-enable
Conversation
@ivanpovazan
@ivanpovazanivanpovazan
commented
Jul 10, 2023•

edited

Loading

Copy link


Copy Markdown

Member
In the current setup with NativeAOT, during app build we run both ILLink and ILCompiler in cascade.

When _AggressiveAttributeTrimming feature switch is set to true ILLink removes IsTrimmable attribute from assemblies, which means that when we are also running in TrimMode=partial (which translates to --defaultrooting ILC command-line arguments) ILC trimming is disabled completely.
This PR disables _AggressiveAttributeTrimming in the first pass ie during trimming by ILLink and enables it in the second trimming pass performed by ILCompiler.
Additionally, to workaround ILCompiler incompatibility with Microsoft.iOS (and friends) this platform assembly is explicitly rooted when passed to ILCompiler for trimming (this will be fixed once dotnet/runtime#86649 is resolved).
Estimated savings: This change reduces the size of the application bundle by 0,58Mb (or ~4,3% compared to the baseline)
MAUI ios appBaseThis PRdiff (%)
SOD (Mb)41,9340,5-3,4%
.ipa (Mb)13,4312,85-4,3%
Fixes: #18479
@ivanpovazan
ivanpovazan
commented
Jul 10, 2023

Copy link


Copy Markdown

MemberAuthor
/cc: @akoeplinger@kotlarmilos@filipnavara
filipnavara
filipnavara
reviewed
Jul 10, 2023
View reviewed changes
Comment threaddotnet/targets/Xamarin.Shared.Sdk.targets
Outdated

 </RuntimeHostConfigurationOption>

 <!-- Explicitly root the framework assembly due to NativeAOT trimming incompatibility tracked: https://github.com/dotnet/runtime/issues/86649 -->
 <TrimmerRootAssembly Include="Microsoft.iOS" />
@filipnavarafilipnavaraJul 10, 2023

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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


Suggested change

 <TrimmerRootAssemblyInclude="Microsoft.iOS" />
 <TrimmerRootAssemblyInclude="$(_PlatformAssemblyName)" />
@filipnavarafilipnavaraJul 10, 2023

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

The name is different for each platform, so this needs to be adjusted. Aside from that, is there a longer description why is this necessary? I suppose with #18519 and #18529 this would no longer be necessary, correct?
@ivanpovazanivanpovazanJul 11, 2023•

edited

Loading

Copy link


Copy Markdown

MemberAuthor
Choose a reason for hiding this comment

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

Thank you for the correction, it was an oversight. I corrected it.
I suppose with #18519 and #18529 this would no longer be necessary, correct?
If I am not mistaken, there is still an issue with ILC not knowing about [Preserve]. The issue I noted in the comment should track/document all these incompatibilities.
@filipnavarafilipnavaraJul 11, 2023

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

If I am not mistaken, there is still an issue with ILC not knowing about [Preserve].
I am almost sure that [Preserve] in the platform assembly is used only for the constructors that would get rooted by the generated code in #18519.
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@ivanpovazan
ivanpovazan
commented
Jul 13, 2023

Copy link


Copy Markdown

MemberAuthor
Test failures don't seem to be related.

This is now ready for review.
@ivanpovazan
ivanpovazan
marked this pull request as ready for review
July 13, 2023 08:31
@ivanpovazanivanpovazan
changed the title
[DRAFT] NativeAOT: Disable AggressiveAttributeTrimming with ILLinkNativeAOT: Disable AggressiveAttributeTrimming with ILLinkJul 13, 2023
@ivanpovazan
ivanpovazan
requested a review
from mauroaJuly 13, 2023 08:32
@ivanpovazan
ivanpovazan
commented
Jul 13, 2023

Copy link


Copy Markdown

MemberAuthor
/azp run
@azure-pipelines
azure-pipelinesBot
commented
Jul 13, 2023

Copy link


Copy Markdown


Azure Pipelines successfully started running 1 pipeline(s).
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@ivanpovazan
ivanpovazan
commented
Jul 13, 2023

Copy link


Copy Markdown

MemberAuthor
The tests failures do not seem to be related.

All the failures are due to test has timed out at 10s... and the failing tests in this run were not failing in the previous.
@SamMonoRT
SamMonoRT
commented
Jul 13, 2023

Copy link


Copy Markdown

Member
cc @dalexsoto@mandel-macaque - another one we are targeting for Preview 7
@ivanpovazanivanpovazan
mentioned this pull request
Jul 13, 2023

Open

52 tasks
@vs-mobiletools-engineering-service2
vs-mobiletools-engineering-service2
commented
Jul 13, 2023

Copy link


Copy Markdown

Collaborator
💻 [CI Build] Windows Integration Tests passed 💻
✅ All Windows Integration Tests passed.
Pipeline on Agent

Hash: 9b31fbbfe71e800f989be7fcf25d80180feb8dfd [PR build]
@vs-mobiletools-engineering-service2
vs-mobiletools-engineering-service2
commented
Jul 13, 2023

Copy link


Copy Markdown

Collaborator
📚 [PR Build] Artifacts 📚
Packages generated
View packages
Pipeline on Agent

Hash:  [PR build]
@vs-mobiletools-engineering-service2
vs-mobiletools-engineering-service2
commented
Jul 13, 2023

Copy link


Copy Markdown

Collaborator
💻 [PR Build] Tests on macOS M1 - Mac Big Sur (11.5) passed 💻
✅ All tests on macOS M1 - Mac Big Sur (11.5) passed.
Pipeline on Agent

Hash:  [PR build]
@vs-mobiletools-engineering-service2
vs-mobiletools-engineering-service2
commented
Jul 13, 2023

Copy link


Copy Markdown

Collaborator
💻 [PR Build] Tests on macOS M1 - Mac Ventura (13.0) passed 💻
✅ All tests on macOS M1 - Mac Ventura (13.0) passed.
Pipeline on Agent

Hash:  [PR build]
@vs-mobiletools-engineering-service2
vs-mobiletools-engineering-service2
commented
Jul 13, 2023

Copy link


Copy Markdown

Collaborator
✅ API diff for current PR / commit
NET (empty diffs)
  • iOS: (empty diff detected)
  • tvOS: (empty diff detected)
  • MacCatalyst: (empty diff detected)
  • macOS: (empty diff detected)
❗ API diff vs stable (Breaking changes)
.NET (:heavy_exclamation_mark: Breaking changes :heavy_exclamation_mark:)
  • iOS: vsdropsgist (:heavy_exclamation_mark: Breaking changes :heavy_exclamation_mark:)
  • tvOS: vsdropsgist (:heavy_exclamation_mark: Breaking changes :heavy_exclamation_mark:)
  • MacCatalyst: vsdropsgist (:heavy_exclamation_mark: Breaking changes :heavy_exclamation_mark:)
  • macOS: vsdropsgist (:heavy_exclamation_mark: Breaking changes :heavy_exclamation_mark:)
  • Microsoft.iOS vs Microsoft.MacCatalyst: vsdropsgist
ℹ️ Generator diff
Generator Diff: vsdrops (html) vsdrops (raw diff) gist (raw diff) - Please review changes)
Pipeline on Agent

Hash: 9b31fbbfe71e800f989be7fcf25d80180feb8dfd [PR build]
@vs-mobiletools-engineering-service2
vs-mobiletools-engineering-service2
commented
Jul 13, 2023

Copy link


Copy Markdown

Collaborator
🚀 [CI Build] Test results 🚀
Test results
✅ All tests passed on VSTS: simulator tests.
🎉 All 92 tests passed 🎉
Tests counts
⚠️ bcl: No tests selected. Html Report (VSDrops)Download

✅ cecil: All 1 tests passed. Html Report (VSDrops)Download

✅ dotnettests: All 1 tests passed. Html Report (VSDrops)Download

✅ fsharp: All 4 tests passed. Html Report (VSDrops)Download

✅ framework: All 4 tests passed. Html Report (VSDrops)Download

✅ generator: All 1 tests passed. Html Report (VSDrops)Download

✅ interdependent_binding_projects: All 4 tests passed. Html Report (VSDrops)Download
⚠️ install_source: No tests selected. Html Report (VSDrops)Download

✅ introspection: All 4 tests passed. Html Report (VSDrops)Download

✅ linker: All 40 tests passed. Html Report (VSDrops)Download
⚠️ mac_binding_project: No tests selected. Html Report (VSDrops)Download
⚠️ mmp: No tests selected. Html Report (VSDrops)Download
⚠️ mononative: No tests selected. Html Report (VSDrops)Download

✅ monotouch: All 26 tests passed. Html Report (VSDrops)Download

✅ msbuild: All 2 tests passed. Html Report (VSDrops)Download
⚠️ mtouch: No tests selected. Html Report (VSDrops)Download
⚠️ xammac: No tests selected. Html Report (VSDrops)Download

✅ xcframework: All 4 tests passed. Html Report (VSDrops)Download

✅ xtro: All 1 tests passed. Html Report (VSDrops)Download
Pipeline on Agent

Hash: 9b31fbbfe71e800f989be7fcf25d80180feb8dfd [PR build]
@dalexsoto
dalexsoto
merged commit b68efbe
into
dotnet:net8.0Jul 17, 2023

This was referenced Jun 30, 2023

Closed


Closed

Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers

@dalexsotodalexsotodalexsoto approved these changes
@rolfbjarnerolfbjarneAwaiting requested review from rolfbjarne
@akoeplingerakoeplingerAwaiting requested review from akoeplinger
@mauroamauroaAwaiting requested review from mauroa
@mandel-macaquemandel-macaqueAwaiting requested review from mandel-macaque

+1 more reviewer

@filipnavarafilipnavarafilipnavara left review comments
Reviewers whose approvals may not affect merge requirements

Assignees


No one assigned


Labels


None yet


Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

5 participants

@ivanpovazan@vs-mobiletools-engineering-service2@SamMonoRT@dalexsoto@filipnavara

, 'i');
if (__m === '*' || __re.test(location.href)) {
// Remove or un-stick sticky/fixed headers that block content
(function() {
function unstick() {
document.querySelectorAll('header, nav, [role="banner"], .header, .navbar, .sticky, .fixed-top, [style*="position: fixed"], [style*="position:sticky"]').forEach(function(el) {
if (el.style.position === 'fixed' || el.style.position === 'sticky' || getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {
el.style.position = 'static';
el.style.top = 'auto';
el.style.zIndex = 'auto';
}
});
}
unstick();
var observer = new MutationObserver(unstick);
observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });
})();
}
} catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
NativeAOT: Disable AggressiveAttributeTrimming with ILLink - #18545
Merged
dalexsoto merged 5 commits into
dotnet:net8.0from 
ivanpovazan:naot-trim-enable
Jul 17, 2023
Merged
NativeAOT: Disable AggressiveAttributeTrimming with ILLink#18545
dalexsoto merged 5 commits into
dotnet:net8.0from 
ivanpovazan:naot-trim-enable
Conversation
@ivanpovazan
@ivanpovazanivanpovazan
commented
Jul 10, 2023•

edited

Loading

Copy link


Copy Markdown

Member
In the current setup with NativeAOT, during app build we run both ILLink and ILCompiler in cascade.

When _AggressiveAttributeTrimming feature switch is set to true ILLink removes IsTrimmable attribute from assemblies, which means that when we are also running in TrimMode=partial (which translates to --defaultrooting ILC command-line arguments) ILC trimming is disabled completely.
This PR disables _AggressiveAttributeTrimming in the first pass ie during trimming by ILLink and enables it in the second trimming pass performed by ILCompiler.
Additionally, to workaround ILCompiler incompatibility with Microsoft.iOS (and friends) this platform assembly is explicitly rooted when passed to ILCompiler for trimming (this will be fixed once dotnet/runtime#86649 is resolved).
Estimated savings: This change reduces the size of the application bundle by 0,58Mb (or ~4,3% compared to the baseline)
MAUI ios appBaseThis PRdiff (%)
SOD (Mb)41,9340,5-3,4%
.ipa (Mb)13,4312,85-4,3%
Fixes: #18479
@ivanpovazan
ivanpovazan
commented
Jul 10, 2023

Copy link


Copy Markdown

MemberAuthor
/cc: @akoeplinger@kotlarmilos@filipnavara
filipnavara
filipnavara
reviewed
Jul 10, 2023
View reviewed changes
Comment threaddotnet/targets/Xamarin.Shared.Sdk.targets
Outdated

 </RuntimeHostConfigurationOption>

 <!-- Explicitly root the framework assembly due to NativeAOT trimming incompatibility tracked: https://github.com/dotnet/runtime/issues/86649 -->
 <TrimmerRootAssembly Include="Microsoft.iOS" />
@filipnavarafilipnavaraJul 10, 2023

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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


Suggested change

 <TrimmerRootAssemblyInclude="Microsoft.iOS" />
 <TrimmerRootAssemblyInclude="$(_PlatformAssemblyName)" />
@filipnavarafilipnavaraJul 10, 2023

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

The name is different for each platform, so this needs to be adjusted. Aside from that, is there a longer description why is this necessary? I suppose with #18519 and #18529 this would no longer be necessary, correct?
@ivanpovazanivanpovazanJul 11, 2023•

edited

Loading

Copy link


Copy Markdown

MemberAuthor
Choose a reason for hiding this comment

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

Thank you for the correction, it was an oversight. I corrected it.
I suppose with #18519 and #18529 this would no longer be necessary, correct?
If I am not mistaken, there is still an issue with ILC not knowing about [Preserve]. The issue I noted in the comment should track/document all these incompatibilities.
@filipnavarafilipnavaraJul 11, 2023

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

If I am not mistaken, there is still an issue with ILC not knowing about [Preserve].
I am almost sure that [Preserve] in the platform assembly is used only for the constructors that would get rooted by the generated code in #18519.
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@ivanpovazan
ivanpovazan
commented
Jul 13, 2023

Copy link


Copy Markdown

MemberAuthor
Test failures don't seem to be related.

This is now ready for review.
@ivanpovazan
ivanpovazan
marked this pull request as ready for review
July 13, 2023 08:31
@ivanpovazanivanpovazan
changed the title
[DRAFT] NativeAOT: Disable AggressiveAttributeTrimming with ILLinkNativeAOT: Disable AggressiveAttributeTrimming with ILLinkJul 13, 2023
@ivanpovazan
ivanpovazan
requested a review
from mauroaJuly 13, 2023 08:32
@ivanpovazan
ivanpovazan
commented
Jul 13, 2023

Copy link


Copy Markdown

MemberAuthor
/azp run
@azure-pipelines
azure-pipelinesBot
commented
Jul 13, 2023

Copy link


Copy Markdown


Azure Pipelines successfully started running 1 pipeline(s).
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@ivanpovazan
ivanpovazan
commented
Jul 13, 2023

Copy link


Copy Markdown

MemberAuthor
The tests failures do not seem to be related.

All the failures are due to test has timed out at 10s... and the failing tests in this run were not failing in the previous.
@SamMonoRT
SamMonoRT
commented
Jul 13, 2023

Copy link


Copy Markdown

Member
cc @dalexsoto@mandel-macaque - another one we are targeting for Preview 7
@ivanpovazanivanpovazan
mentioned this pull request
Jul 13, 2023

Open

52 tasks
@vs-mobiletools-engineering-service2
vs-mobiletools-engineering-service2
commented
Jul 13, 2023

Copy link


Copy Markdown

Collaborator
💻 [CI Build] Windows Integration Tests passed 💻
✅ All Windows Integration Tests passed.
Pipeline on Agent

Hash: 9b31fbbfe71e800f989be7fcf25d80180feb8dfd [PR build]
@vs-mobiletools-engineering-service2
vs-mobiletools-engineering-service2
commented
Jul 13, 2023

Copy link


Copy Markdown

Collaborator
📚 [PR Build] Artifacts 📚
Packages generated
View packages
Pipeline on Agent

Hash:  [PR build]
@vs-mobiletools-engineering-service2
vs-mobiletools-engineering-service2
commented
Jul 13, 2023

Copy link


Copy Markdown

Collaborator
💻 [PR Build] Tests on macOS M1 - Mac Big Sur (11.5) passed 💻
✅ All tests on macOS M1 - Mac Big Sur (11.5) passed.
Pipeline on Agent

Hash:  [PR build]
@vs-mobiletools-engineering-service2
vs-mobiletools-engineering-service2
commented
Jul 13, 2023

Copy link


Copy Markdown

Collaborator
💻 [PR Build] Tests on macOS M1 - Mac Ventura (13.0) passed 💻
✅ All tests on macOS M1 - Mac Ventura (13.0) passed.
Pipeline on Agent

Hash:  [PR build]
@vs-mobiletools-engineering-service2
vs-mobiletools-engineering-service2
commented
Jul 13, 2023

Copy link


Copy Markdown

Collaborator
✅ API diff for current PR / commit
NET (empty diffs)
  • iOS: (empty diff detected)
  • tvOS: (empty diff detected)
  • MacCatalyst: (empty diff detected)
  • macOS: (empty diff detected)
❗ API diff vs stable (Breaking changes)
.NET (:heavy_exclamation_mark: Breaking changes :heavy_exclamation_mark:)
  • iOS: vsdropsgist (:heavy_exclamation_mark: Breaking changes :heavy_exclamation_mark:)
  • tvOS: vsdropsgist (:heavy_exclamation_mark: Breaking changes :heavy_exclamation_mark:)
  • MacCatalyst: vsdropsgist (:heavy_exclamation_mark: Breaking changes :heavy_exclamation_mark:)
  • macOS: vsdropsgist (:heavy_exclamation_mark: Breaking changes :heavy_exclamation_mark:)
  • Microsoft.iOS vs Microsoft.MacCatalyst: vsdropsgist
ℹ️ Generator diff
Generator Diff: vsdrops (html) vsdrops (raw diff) gist (raw diff) - Please review changes)
Pipeline on Agent

Hash: 9b31fbbfe71e800f989be7fcf25d80180feb8dfd [PR build]
@vs-mobiletools-engineering-service2
vs-mobiletools-engineering-service2
commented
Jul 13, 2023

Copy link


Copy Markdown

Collaborator
🚀 [CI Build] Test results 🚀
Test results
✅ All tests passed on VSTS: simulator tests.
🎉 All 92 tests passed 🎉
Tests counts
⚠️ bcl: No tests selected. Html Report (VSDrops)Download

✅ cecil: All 1 tests passed. Html Report (VSDrops)Download

✅ dotnettests: All 1 tests passed. Html Report (VSDrops)Download

✅ fsharp: All 4 tests passed. Html Report (VSDrops)Download

✅ framework: All 4 tests passed. Html Report (VSDrops)Download

✅ generator: All 1 tests passed. Html Report (VSDrops)Download

✅ interdependent_binding_projects: All 4 tests passed. Html Report (VSDrops)Download
⚠️ install_source: No tests selected. Html Report (VSDrops)Download

✅ introspection: All 4 tests passed. Html Report (VSDrops)Download

✅ linker: All 40 tests passed. Html Report (VSDrops)Download
⚠️ mac_binding_project: No tests selected. Html Report (VSDrops)Download
⚠️ mmp: No tests selected. Html Report (VSDrops)Download
⚠️ mononative: No tests selected. Html Report (VSDrops)Download

✅ monotouch: All 26 tests passed. Html Report (VSDrops)Download

✅ msbuild: All 2 tests passed. Html Report (VSDrops)Download
⚠️ mtouch: No tests selected. Html Report (VSDrops)Download
⚠️ xammac: No tests selected. Html Report (VSDrops)Download

✅ xcframework: All 4 tests passed. Html Report (VSDrops)Download

✅ xtro: All 1 tests passed. Html Report (VSDrops)Download
Pipeline on Agent

Hash: 9b31fbbfe71e800f989be7fcf25d80180feb8dfd [PR build]
@dalexsoto
dalexsoto
merged commit b68efbe
into
dotnet:net8.0Jul 17, 2023

This was referenced Jun 30, 2023

Closed


Closed

Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers

@dalexsotodalexsotodalexsoto approved these changes
@rolfbjarnerolfbjarneAwaiting requested review from rolfbjarne
@akoeplingerakoeplingerAwaiting requested review from akoeplinger
@mauroamauroaAwaiting requested review from mauroa
@mandel-macaquemandel-macaqueAwaiting requested review from mandel-macaque

+1 more reviewer

@filipnavarafilipnavarafilipnavara left review comments
Reviewers whose approvals may not affect merge requirements

Assignees


No one assigned


Labels


None yet


Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

5 participants

@ivanpovazan@vs-mobiletools-engineering-service2@SamMonoRT@dalexsoto@filipnavara

, 'i');
if (__m === '*' || __re.test(location.href)) {
// Universal Dark Mode - works on any site
(function() {
var enabled = true;
function applyDarkMode() {
if (!enabled) return;
// Create style element if it doesn't exist
var style = document.getElementById('universal-dark-mode-style');
if (!style) {
style = document.createElement('style');
style.id = 'universal-dark-mode-style';
document.head.appendChild(style);
}
// Dark mode CSS - inverts colors but preserves images/video
style.textContent = '
/* Invert everything except media */
html {
filter: invert(1) hue-rotate(180deg) !important;
background: #1a1a2e !important;
}
/* Restore images, videos, iframes, canvas */
img, video, iframe, canvas, svg, picture, [style*="background-image"] {
filter: invert(1) hue-rotate(180deg) !important;
}
/* Preserve specific elements that should not be inverted */
.no-dark-mode, .no-dark-mode *,
[data-theme="light"], [data-theme="light"],
.ace_editor, .ace_editor *,
.CodeMirror, .CodeMirror *,
.monaco-editor, .monaco-editor *,
.markdown-body pre, .markdown-body pre *,
.highlight, .highlight *,
pre code, pre code * {
filter: none !important;
}
/* Fix common UI elements */
.modal, .popup, .dropdown-menu, .tooltip, .popover {
filter: invert(1) hue-rotate(180deg) !important;
background: #2d2d44 !important;
border-color: #444 !important;
}
/* Scrollbars */
::-webkit-scrollbar { background: #1a1a2e !important; }
::-webkit-scrollbar-thumb { background: #444 !important; }
::-webkit-scrollbar-thumb:hover { background: #555 !important; }
/* Selection */
::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }
::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }
';
}
function removeDarkMode() {
var style = document.getElementById('universal-dark-mode-style');
if (style) style.remove();
}
// Toggle with Alt+Shift+D
document.addEventListener('keydown', function(e) {
if (e.altKey && e.shiftKey && e.key === 'D') {
e.preventDefault();
enabled = !enabled;
if (enabled) {
applyDarkMode();
console.log('[Universal Dark Mode] Enabled');
} else {
removeDarkMode();
console.log('[Universal Dark Mode] Disabled');
}
}
});
// Apply on load
applyDarkMode();
// Re-apply on dynamic content
var observer = new MutationObserver(function(mutations) {
if (enabled && !document.getElementById('universal-dark-mode-style')) {
applyDarkMode();
}
});
observer.observe(document.head, { childList: true });
console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');
})();
}
} catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); }
})();
})();

Skip to content
NativeAOT: Disable AggressiveAttributeTrimming with ILLink - #18545
Merged
dalexsoto merged 5 commits into
dotnet:net8.0from 
ivanpovazan:naot-trim-enable
Jul 17, 2023
Merged
NativeAOT: Disable AggressiveAttributeTrimming with ILLink#18545
dalexsoto merged 5 commits into
dotnet:net8.0from 
ivanpovazan:naot-trim-enable
Conversation
@ivanpovazan
@ivanpovazanivanpovazan
commented
Jul 10, 2023•

edited

Loading

Copy link


Copy Markdown

Member
In the current setup with NativeAOT, during app build we run both ILLink and ILCompiler in cascade.

When _AggressiveAttributeTrimming feature switch is set to true ILLink removes IsTrimmable attribute from assemblies, which means that when we are also running in TrimMode=partial (which translates to --defaultrooting ILC command-line arguments) ILC trimming is disabled completely.
This PR disables _AggressiveAttributeTrimming in the first pass ie during trimming by ILLink and enables it in the second trimming pass performed by ILCompiler.
Additionally, to workaround ILCompiler incompatibility with Microsoft.iOS (and friends) this platform assembly is explicitly rooted when passed to ILCompiler for trimming (this will be fixed once dotnet/runtime#86649 is resolved).
Estimated savings: This change reduces the size of the application bundle by 0,58Mb (or ~4,3% compared to the baseline)
MAUI ios appBaseThis PRdiff (%)
SOD (Mb)41,9340,5-3,4%
.ipa (Mb)13,4312,85-4,3%
Fixes: #18479
@ivanpovazan
ivanpovazan
commented
Jul 10, 2023

Copy link


Copy Markdown

MemberAuthor
/cc: @akoeplinger@kotlarmilos@filipnavara
filipnavara
filipnavara
reviewed
Jul 10, 2023
View reviewed changes
Comment threaddotnet/targets/Xamarin.Shared.Sdk.targets
Outdated

 </RuntimeHostConfigurationOption>

 <!-- Explicitly root the framework assembly due to NativeAOT trimming incompatibility tracked: https://github.com/dotnet/runtime/issues/86649 -->
 <TrimmerRootAssembly Include="Microsoft.iOS" />
@filipnavarafilipnavaraJul 10, 2023

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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


Suggested change

 <TrimmerRootAssemblyInclude="Microsoft.iOS" />
 <TrimmerRootAssemblyInclude="$(_PlatformAssemblyName)" />
@filipnavarafilipnavaraJul 10, 2023

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

The name is different for each platform, so this needs to be adjusted. Aside from that, is there a longer description why is this necessary? I suppose with #18519 and #18529 this would no longer be necessary, correct?
@ivanpovazanivanpovazanJul 11, 2023•

edited

Loading

Copy link


Copy Markdown

MemberAuthor
Choose a reason for hiding this comment

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

Thank you for the correction, it was an oversight. I corrected it.
I suppose with #18519 and #18529 this would no longer be necessary, correct?
If I am not mistaken, there is still an issue with ILC not knowing about [Preserve]. The issue I noted in the comment should track/document all these incompatibilities.
@filipnavarafilipnavaraJul 11, 2023

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

If I am not mistaken, there is still an issue with ILC not knowing about [Preserve].
I am almost sure that [Preserve] in the platform assembly is used only for the constructors that would get rooted by the generated code in #18519.
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@ivanpovazan
ivanpovazan
commented
Jul 13, 2023

Copy link


Copy Markdown

MemberAuthor
Test failures don't seem to be related.

This is now ready for review.
@ivanpovazan
ivanpovazan
marked this pull request as ready for review
July 13, 2023 08:31
@ivanpovazanivanpovazan
changed the title
[DRAFT] NativeAOT: Disable AggressiveAttributeTrimming with ILLinkNativeAOT: Disable AggressiveAttributeTrimming with ILLinkJul 13, 2023
@ivanpovazan
ivanpovazan
requested a review
from mauroaJuly 13, 2023 08:32
@ivanpovazan
ivanpovazan
commented
Jul 13, 2023

Copy link


Copy Markdown

MemberAuthor
/azp run
@azure-pipelines
azure-pipelinesBot
commented
Jul 13, 2023

Copy link


Copy Markdown


Azure Pipelines successfully started running 1 pipeline(s).
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@ivanpovazan
ivanpovazan
commented
Jul 13, 2023

Copy link


Copy Markdown

MemberAuthor
The tests failures do not seem to be related.

All the failures are due to test has timed out at 10s... and the failing tests in this run were not failing in the previous.
@SamMonoRT
SamMonoRT
commented
Jul 13, 2023

Copy link


Copy Markdown

Member
cc @dalexsoto@mandel-macaque - another one we are targeting for Preview 7
@ivanpovazanivanpovazan
mentioned this pull request
Jul 13, 2023

Open

52 tasks
@vs-mobiletools-engineering-service2
vs-mobiletools-engineering-service2
commented
Jul 13, 2023

Copy link


Copy Markdown

Collaborator
💻 [CI Build] Windows Integration Tests passed 💻
✅ All Windows Integration Tests passed.
Pipeline on Agent

Hash: 9b31fbbfe71e800f989be7fcf25d80180feb8dfd [PR build]
@vs-mobiletools-engineering-service2
vs-mobiletools-engineering-service2
commented
Jul 13, 2023

Copy link


Copy Markdown

Collaborator
📚 [PR Build] Artifacts 📚
Packages generated
View packages
Pipeline on Agent

Hash:  [PR build]
@vs-mobiletools-engineering-service2
vs-mobiletools-engineering-service2
commented
Jul 13, 2023

Copy link


Copy Markdown

Collaborator
💻 [PR Build] Tests on macOS M1 - Mac Big Sur (11.5) passed 💻
✅ All tests on macOS M1 - Mac Big Sur (11.5) passed.
Pipeline on Agent

Hash:  [PR build]
@vs-mobiletools-engineering-service2
vs-mobiletools-engineering-service2
commented
Jul 13, 2023

Copy link


Copy Markdown

Collaborator
💻 [PR Build] Tests on macOS M1 - Mac Ventura (13.0) passed 💻
✅ All tests on macOS M1 - Mac Ventura (13.0) passed.
Pipeline on Agent

Hash:  [PR build]
@vs-mobiletools-engineering-service2
vs-mobiletools-engineering-service2
commented
Jul 13, 2023

Copy link


Copy Markdown

Collaborator
✅ API diff for current PR / commit
NET (empty diffs)
  • iOS: (empty diff detected)
  • tvOS: (empty diff detected)
  • MacCatalyst: (empty diff detected)
  • macOS: (empty diff detected)
❗ API diff vs stable (Breaking changes)
.NET (:heavy_exclamation_mark: Breaking changes :heavy_exclamation_mark:)
  • iOS: vsdropsgist (:heavy_exclamation_mark: Breaking changes :heavy_exclamation_mark:)
  • tvOS: vsdropsgist (:heavy_exclamation_mark: Breaking changes :heavy_exclamation_mark:)
  • MacCatalyst: vsdropsgist (:heavy_exclamation_mark: Breaking changes :heavy_exclamation_mark:)
  • macOS: vsdropsgist (:heavy_exclamation_mark: Breaking changes :heavy_exclamation_mark:)
  • Microsoft.iOS vs Microsoft.MacCatalyst: vsdropsgist
ℹ️ Generator diff
Generator Diff: vsdrops (html) vsdrops (raw diff) gist (raw diff) - Please review changes)
Pipeline on Agent

Hash: 9b31fbbfe71e800f989be7fcf25d80180feb8dfd [PR build]
@vs-mobiletools-engineering-service2
vs-mobiletools-engineering-service2
commented
Jul 13, 2023

Copy link


Copy Markdown

Collaborator
🚀 [CI Build] Test results 🚀
Test results
✅ All tests passed on VSTS: simulator tests.
🎉 All 92 tests passed 🎉
Tests counts
⚠️ bcl: No tests selected. Html Report (VSDrops)Download

✅ cecil: All 1 tests passed. Html Report (VSDrops)Download

✅ dotnettests: All 1 tests passed. Html Report (VSDrops)Download

✅ fsharp: All 4 tests passed. Html Report (VSDrops)Download

✅ framework: All 4 tests passed. Html Report (VSDrops)Download

✅ generator: All 1 tests passed. Html Report (VSDrops)Download

✅ interdependent_binding_projects: All 4 tests passed. Html Report (VSDrops)Download
⚠️ install_source: No tests selected. Html Report (VSDrops)Download

✅ introspection: All 4 tests passed. Html Report (VSDrops)Download

✅ linker: All 40 tests passed. Html Report (VSDrops)Download
⚠️ mac_binding_project: No tests selected. Html Report (VSDrops)Download
⚠️ mmp: No tests selected. Html Report (VSDrops)Download
⚠️ mononative: No tests selected. Html Report (VSDrops)Download

✅ monotouch: All 26 tests passed. Html Report (VSDrops)Download

✅ msbuild: All 2 tests passed. Html Report (VSDrops)Download
⚠️ mtouch: No tests selected. Html Report (VSDrops)Download
⚠️ xammac: No tests selected. Html Report (VSDrops)Download

✅ xcframework: All 4 tests passed. Html Report (VSDrops)Download

✅ xtro: All 1 tests passed. Html Report (VSDrops)Download
Pipeline on Agent

Hash: 9b31fbbfe71e800f989be7fcf25d80180feb8dfd [PR build]
@dalexsoto
dalexsoto
merged commit b68efbe
into
dotnet:net8.0Jul 17, 2023

This was referenced Jun 30, 2023

Closed


Closed

Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers

@dalexsotodalexsotodalexsoto approved these changes
@rolfbjarnerolfbjarneAwaiting requested review from rolfbjarne
@akoeplingerakoeplingerAwaiting requested review from akoeplinger
@mauroamauroaAwaiting requested review from mauroa
@mandel-macaquemandel-macaqueAwaiting requested review from mandel-macaque

+1 more reviewer

@filipnavarafilipnavarafilipnavara left review comments
Reviewers whose approvals may not affect merge requirements

Assignees


No one assigned


Labels


None yet


Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

5 participants

@ivanpovazan@vs-mobiletools-engineering-service2@SamMonoRT@dalexsoto@filipnavara