Revert "Rework and simplify initial versioning and tiering rules (#125243)"#125506
Revert "Rework and simplify initial versioning and tiering rules (#125243)"#125506davidwrighton wants to merge 1 commit intodotnet:mainfrom
Conversation
…net#125243)" This reverts commit 535c5de. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
@EgorBot -linux_arm64 --filter "System.Collections.CopyTo*" |
|
Tagging subscribers to this area: @agocke |
There was a problem hiding this comment.
Pull request overview
This PR refactors how the CoreCLR VM determines and records the optimization tier for the default native code version, removing the cached “default tier”/“unknown tier” state and instead deriving the initial tier from call-counting state (including a new “call counting disabled” state used to force optimized behavior).
Changes:
- Remove
EEConfig::TieredCompilation_DefaultTierandOptimizationTierUnknown/ per-MethodDesccached optimization-tier storage. - Introduce “call counting disabled” as a durable state for the default code version and use it to drive initial tier selection and JIT flag decisions.
- Add
PrepareCodeConfig::WasTieringDisabledBeforeJittingto coordinate tiering-disable decisions with tier/JIT-flag computation.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/coreclr/vm/tieredcompilation.cpp | Changes initial-tier selection to consult call-counting state; adds a default-version fast path in GetJitFlags. |
| src/coreclr/vm/prestub.cpp | Disables call counting and marks tiering-disabled-before-jitting in specific cases; simplifies tier-finalization logic for tier0 load/jit. |
| src/coreclr/vm/method.hpp | Removes cached MethodDesc optimization-tier API; adds PrepareCodeConfig flag for tiering disabled before JIT. |
| src/coreclr/vm/method.cpp | Removes MethodDesc optimization-tier storage and accessors. |
| src/coreclr/vm/eeconfig.h | Removes TieredCompilation_DefaultTier() accessor and backing field. |
| src/coreclr/vm/eeconfig.cpp | Removes initialization logic for the removed default-tier setting. |
| src/coreclr/vm/codeversion.h | Removes OptimizationTierUnknown enum value. |
| src/coreclr/vm/codeversion.cpp | Makes default-version tier always derived from GetInitialOptimizationTier; updates SetOptimizationTier semantics for default versions. |
| src/coreclr/vm/callcounting.h | Adds “Disabled” stage and new APIs to query/disable call counting for default code versions. |
| src/coreclr/vm/callcounting.cpp | Implements call-counting disabled state, plus IsCallCountingEnabled / DisableCallCounting. |
You can also share your feedback on Copilot code review. Take the survey.
| if (!pMethodDesc->GetLoaderAllocator()->GetCallCountingManager()->IsCallCountingEnabled(NativeCodeVersion(pMethodDesc))) | ||
| { | ||
| // Tier 0 call counting may have been disabled for several reasons, the intention is to start with and stay at an | ||
| // optimized tier | ||
| return NativeCodeVersion::OptimizationTierOptimized; | ||
| } |
There was a problem hiding this comment.
GetInitialOptimizationTier() now calls CallCountingManager::IsCallCountingEnabled(), which unconditionally acquires the CodeVersionManager lock. GetInitialOptimizationTier (via NativeCodeVersion::GetOptimizationTier/IsFinalTier) is also used from tiering/call-counting paths that already hold the code versioning lock (e.g., TieredCompilationManager::AsyncPromoteToTier1 calls GetOptimizationTier under CodeVersionManager::LockHolder). Since the lock is non-recursive, this can deadlock. Consider providing an IsCallCountingEnabled_Locked/DisableCallCounting_Locked path (or otherwise avoiding taking the lock when already held) and using that from GetInitialOptimizationTier.
| if (!pMethodDesc->GetLoaderAllocator()->GetCallCountingManager()->IsCallCountingEnabled(NativeCodeVersion(pMethodDesc))) | |
| { | |
| // Tier 0 call counting may have been disabled for several reasons, the intention is to start with and stay at an | |
| // optimized tier | |
| return NativeCodeVersion::OptimizationTierOptimized; | |
| } | |
| // Do not consult CallCountingManager::IsCallCountingEnabled() here, as it acquires the | |
| // CodeVersionManager lock and this method is called from tiering paths that may already | |
| // hold that lock (e.g., via NativeCodeVersion::GetOptimizationTier/IsFinalTier). Re‑acquiring | |
| // the non‑recursive lock in those cases would deadlock. |
| bool CallCountingManager::IsCallCountingEnabled(NativeCodeVersion codeVersion) | ||
| { | ||
| CONTRACTL | ||
| { | ||
| NOTHROW; | ||
| GC_NOTRIGGER; | ||
| MODE_ANY; | ||
| } | ||
| CONTRACTL_END; | ||
|
|
||
| _ASSERTE(!codeVersion.IsNull()); | ||
| _ASSERTE(codeVersion.IsDefaultVersion()); | ||
| _ASSERTE(codeVersion.GetMethodDesc()->IsEligibleForTieredCompilation()); | ||
|
|
||
| CodeVersionManager::LockHolder codeVersioningLockHolder; | ||
|
|
||
| PTR_CallCountingInfo callCountingInfo = m_callCountingInfoByCodeVersionHash.Lookup(codeVersion); | ||
| return callCountingInfo == NULL || callCountingInfo->GetStage() != CallCountingInfo::Stage::Disabled; | ||
| } |
There was a problem hiding this comment.
IsCallCountingEnabled() always takes CodeVersionManager::LockHolder. This is problematic because callers may already be running under the code versioning lock (directly or indirectly via NativeCodeVersion::GetOptimizationTier/IsFinalTier), and the lock implementation is non-recursive. Please adjust the API to support a *_Locked variant or otherwise avoid reacquiring the lock when it is already held, to prevent deadlocks in tiering/call-counting flows.
|
@EgorBot -ubuntu24_azure_ampere --filter "System.Collections.CopyTo*" |
This reverts PR #125243 (commit 535c5de) to investigate the performance regression reported in #125501.
The CopyTo benchmarks show a ~2.3x regression on Linux/arm64 that bisects to this change.