Gc arrays

[AndyAyersMS]

No description provided.

[dotnet-policy-service]

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

[jakobbotsch]

I was hoping we would not end up with another "layout kind", instead adding full support for custom layouts and removing block layouts. It would benefit more places than just array stack allocation.

[AndyAyersMS]

Can you say more about what you had in mind? Some kind of ability to compose layouts from other layouts, including repeating elements?

[jakobbotsch]

Today we have CreateObjLayout(CORINFO_CLASS_HANDLE) and CreateBlkLayout(unsigned size). What I was thinking is to delete block layouts in favor of a CreateCustomLayout(unsigned size, CorInfoGCType* slotGCTypes), potentially with a way to set the significant segments (non-padding) as well. Block layouts would be represented as CreateCustomLayout(size, nullptr). Array layouts would be represented via repeating the GC pointer types and padding of another layout. Box layouts would be created by adding an extra non-pointer at the beginning of another layout (removing the need for getTypeForBoxOnStack). Physical promotion would also benefit as it could start representing remainders with GC pointers, so it would need to copy less in some cases.

The custom layouts could almost subsume even object layouts, however we do have a few cases left where we need to get the CORINFO_CLASS_HANDLE out of a layout. If we got rid of those we could unify everything on the custom layouts.

I think the main work would be in going around and replacing the ClassLayout::IsBlockLayout checks with either "is custom layout" (no class handle) or "has GC pointers" checks. Of course the devil might be in the details...

[jakobbotsch]

We could certainly also introduce some ClassLayoutBuilder class to allow easily composing a new custom class layout from various building blocks (like repeating a layout some number of times)

[AndyAyersMS]

One hangup in moving to a more general construction is the hashing scheme -- without a class handle as a convenient key, it seems like we may need to compute a full layout in order to realize we have created a duplicate, at which point the value of duplicate detection is minimal, as we've likely already burned the memory.

For block layouts this is not a concern, since their only interesting property is their size, but once there are gc slots scattered about...

Perhaps it's rare to create duplicate custom layouts so this is a non-problem (though this PR currently relies on it since it was simple to do). Or we perhaps we just don't worry about it and waste some memory representing duplicates?

[jakobbotsch]

Perhaps it's rare to create duplicate custom layouts so this is a non-problem (though this PR currently relies on it since it was simple to do). Or we perhaps we just don't worry about it and waste some memory representing duplicates?

It sounds ok to me to waste a bit of memory. ClassLayout itself already does that -- it could represent the GC pointers array using much less memory than it is doing now, since each element requires only 2 bits. With a ClassLayoutBuilder using small-size optimization most layouts probably wouldn't require any dynamic allocation if they hit the cache for custom layouts.
Not sure I would worry about it though... We probably aren't going to be creating that many custom layouts, and when we do it's because some optimization is triggering anyway.

I'm not sure that the caching is as much about saving memory as it is about having a canonical index/ID for each layout. See this comment:

runtime/src/coreclr/jit/layout.cpp

Lines 15 to 19 in c74440f

	// Each layout is assigned a number, starting with TYP_UNKNOWN + 1. This way one could use a single
	// unsigned value to represent the notion of type - values below TYP_UNKNOWN are var_types and values
	// above it are struct layouts.
	static constexpr unsigned ZeroSizedBlockLayoutNum = TYP_UNKNOWN + 1;
	static constexpr unsigned FirstLayoutNum = TYP_UNKNOWN + 2;

We have never really used this capability, though.

[AndyAyersMS]

SPMI has too many missed contexts, also this depends on the base array PR... need to wait for that to go in to get a better read on this.

[AndyAyersMS]

Still lots of missed contexts, will look into juicing up SPMI collections with extra queries.

[AndyAyersMS]

@MihuBot

[AndyAyersMS]

So how does this sound: a public API for ClassLayout like it is today, plus constructor methods:

    static ClassLayout* Create(Compiler* compiler, CORINFO_CLASS_HANDLE classHandle);
    static ClassLayout* CreateBox(Compiler* compiler, ClassLayout* payloadLayout);
    static ClassLayout* CreateBlock(Compiler* compiler, unsigned length);
    static ClassLayout* CreateCustom(Compiler* compiler, unsigned length);
    static void AddRepeatingElements(unsigned offset, ClassLayout* elementLayout, unsigned count);
    static void AddGCFields(unsigned offset, unsigned count);
    void Finalize();

where a custom layout allows adding GC references until Finalize is called, and the others are finalized immediately.

The CustomLayoutTable can then move behind the scenes to memoize construction of the first kind of layout; the rest we just create on demand (or keep memoizing blocks too if it matters).

The Add methods do some minimal checking to ensure a GC field set by a previous call is not getting toggled off by a later one, or perhaps in debug that all of the Add operations are disjoint.

If this sounds like what you had in mind I will work it up as a separate change and then come back to building this on top.

[jakobbotsch]

So how does this sound: a public API for ClassLayout like it is today, plus constructor methods:

    static ClassLayout* Create(Compiler* compiler, CORINFO_CLASS_HANDLE classHandle);
    static ClassLayout* CreateBox(Compiler* compiler, ClassLayout* payloadLayout);
    static ClassLayout* CreateBlock(Compiler* compiler, unsigned length);
    static ClassLayout* CreateCustom(Compiler* compiler, unsigned length);
    static void AddRepeatingElements(unsigned offset, ClassLayout* elementLayout, unsigned count);
    static void AddGCFields(unsigned offset, unsigned count);
    void Finalize();

where a custom layout allows adding GC references until Finalize is called, and the others are finalized immediately.

The CustomLayoutTable can then move behind the scenes to memoize construction of the first kind of layout; the rest we just create on demand (or keep memoizing blocks too if it matters).

The Add methods do some minimal checking to ensure a GC field set by a previous call is not getting toggled off by a later one, or perhaps in debug that all of the Add operations are disjoint.

If this sounds like what you had in mind I will work it up as a separate change and then come back to building this on top.

I would suggest a new ClassLayoutBuilder type to house the methods used to construct the layout. That type could act as the key for the memoized custom layouts, plus I expect to PR some follow-ups to also be able to build the padding/non-padding segments and store those in the ClassLayout.