Skip to content

Conversation

@pavelsavara
Copy link
Member

@pavelsavara pavelsavara commented Jan 19, 2026

Contributes to #123224

@pavelsavara pavelsavara added this to the 11.0.0 milestone Jan 19, 2026
@pavelsavara pavelsavara self-assigned this Jan 19, 2026
@pavelsavara pavelsavara added arch-wasm WebAssembly architecture os-browser Browser variant of arch-wasm area-CodeGen-Interpreter-coreclr labels Jan 19, 2026
@pavelsavara pavelsavara changed the title [browser][coreCLR] [browser][coreCLR] IsCoreCLRInterpreter and runtimeVariant: coreclrinterpreter Jan 19, 2026
@dotnet-policy-service
Copy link
Contributor

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

Copy link
Member

@jkoritzinsky jkoritzinsky left a comment

Choose a reason for hiding this comment

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

runtimevariant was added for Mono because the runtime would be built fundamentally differently depending on variant. That's not the case for how we build CoreCLR, so I don't think using it applies in the same way.

@pavelsavara pavelsavara changed the title [browser][coreCLR] IsCoreCLRInterpreter and runtimeVariant: coreclrinterpreter [browser][coreCLR] use IsCoreCLRInterpreter Jan 19, 2026
@pavelsavara pavelsavara marked this pull request as ready for review January 19, 2026 18:46
Copilot AI review requested due to automatic review settings January 19, 2026 18:46
@pavelsavara pavelsavara enabled auto-merge (squash) January 19, 2026 18:47
Copy link
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

This PR contributes to issue #123224 by introducing a unified IsInterpreter property in PlatformDetection that combines checks for both Mono and CoreCLR interpreters, and updates test code to use this new property where appropriate.

Changes:

  • Introduces IsInterpreter and IsNotInterpreter properties in PlatformDetection.cs that combine both IsMonoInterpreter and IsCoreClrInterpreter checks
  • Updates test files to use the new IsInterpreter property or add IsCoreClrInterpreter checks where appropriate
  • Improves consistency in interpreter detection across the test suite

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File Description
src/libraries/Common/tests/TestUtilities/System/PlatformDetection.cs Adds IsInterpreter and IsNotInterpreter properties that combine both interpreter types
src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Utf8JsonReaderTests.cs Updates TestDepth test to use IsInterpreter instead of IsMonoInterpreter
src/libraries/System.Text.Json/tests/System.Text.Json.Tests/AssemblyInfo.cs Updates ActiveIssue attribute to use IsInterpreter instead of IsMonoInterpreter
src/libraries/System.Runtime/tests/System.Runtime.Tests/System/Runtime/JitInfoTests.cs Adds IsCoreClrInterpreter checks alongside existing IsMonoInterpreter checks

}

if (PlatformDetection.IsMonoInterpreter)
if (PlatformDetection.IsMonoInterpreter || PlatformDetection.IsCoreClrInterpreter)
Copy link
Member

Choose a reason for hiding this comment

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

CoreCLR interpreter is compiling methods just like a JIT. Is it expected that the number of methods compiled by interpreter JIT is not included in JitInfo.GetCompiledMethodCount?

long afterCompiledMethodCount = System.Runtime.JitInfo.GetCompiledMethodCount();

if (PlatformDetection.IsMonoInterpreter || PlatformDetection.IsMonoAOT || PlatformDetection.IsReadyToRunCompiled)
if (PlatformDetection.IsMonoInterpreter || PlatformDetection.IsCoreClrInterpreter || PlatformDetection.IsMonoAOT || PlatformDetection.IsReadyToRunCompiled)
Copy link
Member

Choose a reason for hiding this comment

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

I would expect IsReadyToRunCompiled condition to sufficient here. Why is it not enough?

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

Labels

arch-wasm WebAssembly architecture area-CodeGen-Interpreter-coreclr os-browser Browser variant of arch-wasm

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants