Skip to content

Delete ArrayEEClass - compute multidim array rank from BaseSize#124944

Merged
jkotas merged 11 commits intomainfrom
copilot/remove-array-ee-class
Feb 28, 2026
Merged

Delete ArrayEEClass - compute multidim array rank from BaseSize#124944
jkotas merged 11 commits intomainfrom
copilot/remove-array-ee-class

Conversation

Copy link
Contributor

Copilot AI commented Feb 27, 2026

ArrayEEClass (ArrayClass) stored only m_rank — a single byte redundant with information already encoded in MethodTable::m_baseSize. This PR removes the class entirely and derives rank from BaseSize, matching NativeAOT's approach.

Changes

CoreCLR VM (C++)

  • class.h: Remove ArrayClass entirely. Remove PTR_ArrayClass typedef (no longer needed). Add Module::CreateArrayMethodTable as friend of EEClass (its constructor is protected, and the function now calls ::new (pMemory) EEClass() directly).
  • methodtable.inl: MethodTable::GetRank() now computes rank from BaseSize:
    // SZARRAY fast path unchanged; multidim:
    DWORD boundsSize = GetBaseSize() - ARRAYBASE_BASESIZE;
    return boundsSize / (sizeof(DWORD) * 2);
    This works because CreateArrayMethodTable sets BaseSize = ARRAYBASE_BASESIZE + Rank * sizeof(DWORD) * 2 for ELEMENT_TYPE_ARRAY. The DATA_ALIGNMENT > 4 padding path on 32-bit is dead code on all supported platforms.
  • array.cpp: GenerateArrayAccessorCallSig and InitArrayMethodDesc become local static void functions (were ArrayClass members). Allocation uses sizeof(EEClass) and EEClass* directly; SetRank() call removed.
  • object.inl: GetLowerBoundsOffset uses pMT->GetRank() directly.
  • datadescriptor.inc: Remove ArrayClass type block; add CDAC_GLOBAL(ArrayBaseSize, uint32, ARRAYBASE_BASESIZE).
  • methodtable.cpp: Remove redundant if (IsArray()) DAC memory enumeration block (now handled uniformly by EEClass::EnumMemoryRegions). Simplify array cast check to use !pTargetMT->IsMultiDimArray() instead of GetInternalCorElementType() == ELEMENT_TYPE_SZARRAY.
  • methodtable.h, ceeload.h: Delete all friend class NativeImageDumper; declarations throughout the repo (4 occurrences).

Managed cDAC

  • Remove DataType.ArrayClass, Data.ArrayClass.
  • RuntimeTypeSystem_1.IsArray() computes rank from BaseSize using the new ArrayBaseSize global (Constants.Globals.ArrayBaseSize).

cDAC Tests

  • Mock descriptors no longer allocate ArrayClass; multidim array MethodTable mock now sets BaseSize = ArrayBaseSize + rank * 2 * sizeof(int) (previously always used ArrayBaseSize, which was inconsistent with the real runtime).
  • ObjectTests data-pointer assertions updated: for a rank-R multidim array, data starts at address + ArrayBaseSize + R * 2 * sizeof(int).
  • Added ValidateMultidimArrayRank test as a [Theory] over all standard architectures, directly verifying IsArray() returns correct rank (4 and 1) from BaseSize.

Documentation

  • docs/design/datacontracts/RuntimeTypeSystem.md: Removed the ArrayClass | Rank data descriptor row, added ArrayBaseSize to the globals table (with description: "The base size of an array object; used to compute multidimensional array rank from MethodTable::BaseSize"), and updated the IsArray pseudocode to derive multidimensional array rank from BaseSize instead of reading from ArrayClass.

✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

@dotnet-policy-service
Copy link
Contributor

Tagging subscribers to this area: @agocke
See info in area-owners.md if you want to be subscribed.

Copilot AI and others added 2 commits February 27, 2026 06:30
…ns local static

Co-authored-by: jkotas <6668460+jkotas@users.noreply.github.com>
…ay rank test

Co-authored-by: jkotas <6668460+jkotas@users.noreply.github.com>
Copilot AI changed the title [WIP] Remove ArrayEEClass and update MethodTable implementation Delete ArrayEEClass - compute multidim array rank from BaseSize Feb 27, 2026
Copy link
Member

@jkotas jkotas left a comment

Choose a reason for hiding this comment

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

@jkotas
Copy link
Member

jkotas commented Feb 27, 2026

@copilot More feedback above

…ageDumper friends, remove redundant IsArray DAC block

Co-authored-by: jkotas <6668460+jkotas@users.noreply.github.com>
…cdac

Co-authored-by: jkotas <6668460+jkotas@users.noreply.github.com>
Co-authored-by: jkotas <6668460+jkotas@users.noreply.github.com>
@jkotas jkotas marked this pull request as ready for review February 27, 2026 15:29
@jkotas
Copy link
Member

jkotas commented Feb 27, 2026

@max-charlamb PTLA - small data contract simplification that makes NativeAOT and CoreCLR more similar.

Also, could you please take look at the runtime-diagnostics failures? They do not appear related to my change.

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

Removes the redundant CoreCLR ArrayClass/ArrayEEClass (which only stored array rank) and derives multidimensional array rank from MethodTable::BaseSize, aligning CoreCLR with the existing NativeAOT approach. Updates cDAC (native + managed) and its tests to match the new layout/contract shape.

Changes:

  • CoreCLR VM: delete ArrayClass, refactor array helper methods to file-local statics, and compute MethodTable::GetRank() from BaseSize.
  • cDAC: remove ArrayClass data/type usage and compute array rank from BaseSize (plus a new exported global for ARRAYBASE_BASESIZE).
  • Tests: adjust mock descriptors/object layout math and add coverage validating multidim rank derivation.

Reviewed changes

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

Show a summary per file
File Description
src/coreclr/vm/array.cpp Refactors former ArrayClass helpers to static functions; allocates EEClass directly for arrays.
src/coreclr/vm/class.h Removes ArrayClass definition; adds Module::CreateArrayMethodTable friend to allow placement-new EEClass.
src/coreclr/vm/classcompat.h Removes ArrayClass forward decl.
src/coreclr/vm/ceeload.h Removes NativeImageDumper friend declarations.
src/coreclr/vm/datadescriptor/datadescriptor.inc Removes ArrayClass type; adds ArrayBaseSize global exporting ARRAYBASE_BASESIZE.
src/coreclr/vm/methodtable.cpp Simplifies array casting logic and DAC memory enumeration now that ArrayClass is gone.
src/coreclr/vm/methodtable.h Removes ArrayClass forward decl and NativeImageDumper friends.
src/coreclr/vm/methodtable.inl Implements MethodTable::GetRank() using BaseSize - ARRAYBASE_BASESIZE.
src/coreclr/vm/object.inl Computes lower-bounds offset using pMT->GetRank() directly.
src/coreclr/vm/vars.hpp Removes ArrayClass forward decl.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/DataType.cs Removes DataType.ArrayClass.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Constants.cs Adds Globals.ArrayBaseSize.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/RuntimeTypeSystem_1.cs Computes multidim rank from BaseSize using Globals.ArrayBaseSize.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Data/EEClass.cs Deletes managed ArrayClass data reader.
src/native/managed/cdac/tests/ContractDescriptor/TargetTests.SubDescriptors.cs Removes ArrayClass size-only descriptor entry.
src/native/managed/cdac/tests/MethodTableTests.cs Adds ValidateMultidimArrayRank coverage for rank derivation.
src/native/managed/cdac/tests/MockDescriptors/MockDescriptors.cs Removes ArrayClassFields descriptor.
src/native/managed/cdac/tests/MockDescriptors/MockDescriptors.Object.cs Updates mock array BaseSize to encode rank for multidim arrays; uses EEClass directly.
src/native/managed/cdac/tests/MockDescriptors/MockDescriptors.RuntimeTypeSystem.cs Removes AddArrayClass; adds Globals.ArrayBaseSize to the mock target.
src/native/managed/cdac/tests/ObjectTests.cs Updates expected multidim array data pointer to account for rank-encoded bounds in BaseSize.

@max-charlamb
Copy link
Member

@copilot, please update RuntimeTypeSystem.md docs with the relevant changes.

…bal, update IsArray pseudocode

Co-authored-by: max-charlamb <44248479+max-charlamb@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Feb 27, 2026

@copilot, please update RuntimeTypeSystem.md docs with the relevant changes.

Done in 6e4b409. Updated RuntimeTypeSystem.md to: remove the ArrayClass | Rank data descriptor row, add ArrayBaseSize to the globals table, and update the IsArray pseudocode to derive multidim rank from BaseSize instead of reading from ArrayClass.

@max-charlamb
Copy link
Member

@copilot , please fix the build break in linux-x64 checked CLR_Tools_Tests caused by the modified cDAC unit tests

Copy link
Member

@max-charlamb max-charlamb left a comment

Choose a reason for hiding this comment

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

lgtm from a cDAC perspective once the cDAC unit tests pass CI

… test method

Co-authored-by: max-charlamb <44248479+max-charlamb@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Feb 27, 2026

@copilot , please fix the build break in linux-x64 checked CLR_Tools_Tests caused by the modified cDAC unit tests

Fixed in 0e7648e. The ValidateMultidimArrayRank method was missing [Theory] and [ClassData(typeof(MockTarget.StdArch))] attributes, triggering xUnit1013. All 916 cDAC tests pass locally.

@jkotas
Copy link
Member

jkotas commented Feb 28, 2026

@copilot use ci-analysis skill to analyze the CI results. Are the failures related to this change?

Copy link
Contributor Author

Copilot AI commented Feb 28, 2026

@copilot use ci-analysis skill to analyze the CI results. Are the failures related to this change?

The CI analysis script can't reach Azure DevOps from this sandbox (no auth), but based on what's visible:

Runtime-diagnostics failuresNot related to this PR. These were caused by a known Azure Pipelines regression in the "Publish Test Results" task (upstream issue). The workaround was already merged to main as 7356734a and is included in this branch via the merge commit.

linux-x64 checked CLR_Tools_Tests failureWas related to this PR. Fixed in 0e7648ee: the new ValidateMultidimArrayRank test was missing [Theory] and [ClassData(typeof(MockTarget.StdArch))] attributes (xUnit1013). All 916 cDAC tests pass locally after the fix.

No files changed by this PR overlap with the runtime-diagnostics pipeline.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • https://api.github.com/graphql
    • Triggering command: /usr/bin/gh /usr/bin/gh pr checks 124944 --repo dotnet/runtime (http block)

If you need me to access, download, or install something from one of these locations, you can either:

@jkotas
Copy link
Member

jkotas commented Feb 28, 2026

/ba-g iOS timeout, cDAC failures are same as on other PRs

@jkotas jkotas merged commit faf542a into main Feb 28, 2026
109 of 114 checks passed
@jkotas jkotas deleted the copilot/remove-array-ee-class branch February 28, 2026 16:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants