Skip to content

Disable ICorProfiler on non-desktop and WASM platforms#126550

Merged
AaronRobinsonMSFT merged 6 commits intodotnet:mainfrom
AaronRobinsonMSFT:disable-profiler-on-mobile
Apr 8, 2026
Merged

Disable ICorProfiler on non-desktop and WASM platforms#126550
AaronRobinsonMSFT merged 6 commits intodotnet:mainfrom
AaronRobinsonMSFT:disable-profiler-on-mobile

Conversation

@AaronRobinsonMSFT
Copy link
Copy Markdown
Member

@AaronRobinsonMSFT AaronRobinsonMSFT commented Apr 4, 2026

Note

This PR description was generated with the help of GitHub Copilot.

Refactor FEATURE_CORPROFILER in clrfeatures.cmake to consolidate the ICorProfiler feature flag logic:

  • Disable ICorProfiler on non-desktop targets — Android, Mac Catalyst, iOS, and tvOS — in addition to WASM, since profiling is not supported on these platforms.
  • Make the flag overridable by wrapping it in if(NOT DEFINED FEATURE_CORPROFILER), allowing it to be set from the command line.
  • Remove the redundant WASM override that previously set FEATURE_CORPROFILER 0 separately.

Refactor FEATURE_CORPROFILER in clrfeatures.cmake to disable the
ICorProfiler feature on Android, Mac Catalyst, iOS, and WASM targets.
Previously the profiler was unconditionally enabled and then only
explicitly disabled for WASM. This consolidates the logic into a
single guard that excludes all unsupported platforms and makes the
flag overridable from the command line.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings April 4, 2026 18:54
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Refactors the CoreCLR CMake feature-flag logic for FEATURE_CORPROFILER so ICorProfiler support is disabled by default on unsupported targets (mobile + WASM) while allowing command-line overrides.

Changes:

  • Wrap FEATURE_CORPROFILER defaulting in if(NOT DEFINED FEATURE_CORPROFILER) to allow CLI override.
  • Enable FEATURE_CORPROFILER only for non-WASM, non-mobile targets (Android, iOS, Mac Catalyst).
  • Remove the prior WASM-specific override that forcibly set FEATURE_CORPROFILER 0.

AaronRobinsonMSFT and others added 2 commits April 4, 2026 12:13
Copilot AI review requested due to automatic review settings April 4, 2026 19:18
@AaronRobinsonMSFT AaronRobinsonMSFT changed the title Disable ICorProfiler on mobile and WASM platforms Disable ICorProfiler on non-desktop and WASM platforms Apr 4, 2026
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.

@AaronRobinsonMSFT
Copy link
Copy Markdown
Member Author

@jkotas @noahfalk I'm updating the logic here based on #126493 (comment).

@jkotas
Copy link
Copy Markdown
Member

jkotas commented Apr 4, 2026

@dotnet/dotnet-diag Do we have any scenarios for ICorProfiler on iOS and Android? Is anything going to break if we drop it?

@AaronRobinsonMSFT
Copy link
Copy Markdown
Member Author

@thaystg Please let me know if any of your prototypes need this to a degree that we should defer limiting it for the related platform.

@AaronRobinsonMSFT AaronRobinsonMSFT added the arch-wasm WebAssembly architecture label Apr 4, 2026
@thaystg
Copy link
Copy Markdown
Member

thaystg commented Apr 6, 2026

@thaystg Please let me know if any of your prototypes need this to a degree that we should defer limiting it for the related platform.

Correct, right now we totally depend on it. @noahfalk any suggestion about it?

@thaystg
Copy link
Copy Markdown
Member

thaystg commented Apr 6, 2026

We need this enabled at least until preview 3.

@noahfalk
Copy link
Copy Markdown
Member

noahfalk commented Apr 7, 2026

@dotnet/dotnet-diag Do we have any scenarios for ICorProfiler on iOS and Android? Is anything going to break if we drop it?

Other than its use as the in-proc debugger loading mechanism I'm not aware of any other scenarios that require it. @thaystg - are you aware of anything else that requires it?

@noahfalk any suggestion about it?

What if we turned it off for WASM now, and turn it off for Android/iOS in a future preview once we've resolved the in-proc debugging loading?

@noahfalk
Copy link
Copy Markdown
Member

noahfalk commented Apr 7, 2026

[@jkotas] (#126493 (comment)) The question to ask is whether we want FEATURE_CORPROFILER for mobile platforms

I'm not aware of any 3rd parties specifically asking/planning to use it and it hasn't been there historically on Mono. I'd be happy to have it off for now and wait to see some clear evidence that profiler vendors want it on these platforms.

Copilot AI review requested due to automatic review settings April 7, 2026 15:04
@AaronRobinsonMSFT
Copy link
Copy Markdown
Member Author

What if we turned it off for WASM now, and turn it off for Android/iOS in a future preview once we've resolved the in-proc debugging loading?

Done.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.

Copilot AI review requested due to automatic review settings April 8, 2026 04:27
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

@AaronRobinsonMSFT
Copy link
Copy Markdown
Member Author

/ba-g Unrelated failures

@AaronRobinsonMSFT AaronRobinsonMSFT merged commit 0fc6976 into dotnet:main Apr 8, 2026
111 of 115 checks passed
@github-project-automation github-project-automation bot moved this to Done in AppModel Apr 8, 2026
@AaronRobinsonMSFT AaronRobinsonMSFT deleted the disable-profiler-on-mobile branch April 8, 2026 18:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

6 participants