Support metaclasses in HPy.

[fangerer]

This PR resolves #296 and is one of the big pieces that is required for migration of NumPy.

Goal of this PR: Support metaclasses for HPy types.

Before version 3.12, CPython does not support specifying metaclasses for heap types but only for static types.
We can still support that by creating a temporary heap type using PyType_FromSpecWithBases and then we manually allocate another heap type using the provided metaclass' tp_alloc. We then copy everything from the temporary heap type to the final one, reset some carefully picked slots and initialize it with PyType_Ready.
The temporary heap type is then trashed.

However, making that work with pure HPy types wasn't that trivial because a metaclass usually embeds the heap type struct like this:

typedef struct {
    PyHeapTypeObject super; // sizeof(PyHeapTypeObject) == 880
    int x;
    int y;
    // ....
} LegacyMetaClass;

The problem is that if we specify a pure HPy type and use it as metaclass, we would still just have following struct:

typedef struct {
    int x;
    int y;
    // ...
} MetaClass;

Since from an HPy point of view, the runtime's heap type is opaque, you cannot embed that in your structure and HPy assumes that it's just a bare PyObject and so the used offset is just sizeof(PyObject).

However, each metaclass needs to inherit from PyType_Type (directly or indirectly). I use that fact and iterate over the bases to determine the largest basicsize. That is then used as an offset.
Furthermore, we now don't have a fixed offset for the pure HPy data pointer and so we need to store that in the HPy type's extra info.
This makes the *_AsStruct calls slightly more expensive because we need to load the offset from the object type's extra info.
I think, this change is not only a fix for metaclasses but in general a fix to be able to inherit from any opaque type. For example, an HPy extension could define a type that inherits from a foreign HPy extension's type which also specifies a basicsize.
Furthermore, as far as I know, the situation is similar in the C API when using the limited API. The structs are opaque and if one wants to inherit from a type, he needs to add the base type's basicsize.

[steve-s]

From my POV this looks good modulo the failing CI jobs and is an important step forward for NumPy porting effort. It would be great if someone else could take a look too (@antocuni, @hodgestar, @rlamy, ... :-))

[fangerer]

Yes, I'm sorry. MSVC is a bit more pedantic than clang or gcc.
I took the opportunity to write a test which is not straight forward since we should not make too many assumptions.

[antocuni]

wow, this was a tremendous amount of work, good job @fangerer and @steve-s 👍.
The PR looks good in general, but before merging I would like to explore whether there are other ways to avoid paying the penalty in HPy_AsStruct.

HPy_AsStruct will be called a lot, and we should try at all costs to avoid making it unnecessarily slow, especially considering that it's a penalty that we are going to pay also in CPython mode, which in theory should be 0 costs.
I also have the feeling that we are going in circles, because when we proposed #83, we did it specifically to avoid paying the cost of this kind of "dynamic cast".

I think there is a possible solution which came to my mind while reading this PR, so let me explain it here although it might be wrong.

If I understand correctly, the problem comes when subclassing a builtin type which is not PyObject. But the key realization is that there is only a finite number of those: PyHeapTypeObject, PyLongObject, PyDictObject and probably a handful more. The relevant issue is #169.

Currently, we distinguish between "pure types" and "legacy types". I propose to treat specially all built-in type that can be subclassed (or at least, the ones for which we care about performance). So, we will have:

HPy_AsStruct(); // works only for pure types
HPy_AsStruct_Generic(); // this is the "slow" version, which dynamically looks up the payload_offset. Maybe we should call it HPy_AsStruct_Dynamic? Or _Slow?
HPy_AsStruct_Legacy_PyObject();
HPy_AsStruct_Legacy_PyHeapTypeObject();
...

HPyType_DEFINE_HELPERS will be able to use the most appropriate version, and probably we can teach the debug mode to detect the case in which we are using a specialized version which does not match what's written in payload_offset.

Note that we still need the "generic" slow version for cases in which we don't know upfront the type, but this is already the case in the current master: we have _pyobj_as_struct which does exactly that and it's called from a couple of internal places:

hpy/hpy/devel/src/runtime/ctx_type.c

Lines 73 to 81 in d262f43

	static void *_pyobj_as_struct(PyObject *obj)
	{
	if (_is_pure_HPyType(Py_TYPE(obj))) {
	return _HPy_PyObject_Payload(obj);
	}
	else {
	return obj;
	}
	}

Also, if we decide to go down that route, we can probably remove the is_pure flag:

typedef struct {
    uint16_t magic;
    HPyFunc_traverseproc tp_traverse_impl;
    HPyFunc_destroyfunc tp_destroy_impl;
    bool is_pure;
    HPy_ssize_t payload_offset;
    char name[];
} HPyType_Extra_t;

I think that payload_offset should be enough: a type is pure IIF payload_offset == 0.

[antocuni]

as you have already noticed, we already have a mechanism to align an arbitrary payload:
https://github.com/fangerer/hpy.git/blob/f337049cb17d2538b0439c5ac45beca0f0470f08/hpy/devel/include/hpy/runtime/ctx_type.h#L25-40

I'm happy to use this new logic, but then we should probably kill _HPy_FullyAlignedSpaceForPyObject_HEAD and use the new logic also there?

[antocuni]

this might be nitpick, but was wrong with _HPyType_EXTRA(tp)->is_pure? It looked more natural and easier to read to me, and it doesn't require to introduce a new long macro for every field that we will add to HPyType_Extra_t.
Am I missing something obvious for why the new macro was needed?

[antocuni]

I understand why you ended up with this solution of using a payload, but I would really like to avoid this at all costs.
I will write a more extensive explanation in the main thread, because it's better to discuss there than in a small side comment.

[antocuni]

I need to think more to really understand what's happening here, but a temporary hackish solution for "old" CPython versions sounds good to me.
I wonder whether we should move it to its own file so that we can easily exclude it from compilation with newer Pythons.
I know that in theory we could guard it with #ifdef also in this case, but I think it's too long and would be confusing.

[steve-s]

If I understand correctly, the problem comes when subclassing a builtin type which is not PyObject. But the key realization is that there is only a finite number of those: PyHeapTypeObject, PyLongObject, PyDictObject and probably a handful more. The relevant issue is #169.

I though the problem here was that we have also custom subclasses. However, now I realized we do not have this problem here. At the same time, we still need to know the right offset, and even for pure types (or only for pure types actually). Let's have some example for discussion:

struct {
   PyHeapTypeObject super;
   int data;
} DTypeMeta;

static HPyType_Spec DTypeMeta_spec = {
  .basicsize = sizeof(DTypeMeta)
  // ...
};

struct {
   PyObject_HEAD
   int other_data;
} DType;

static HPyType_Spec DType_spec = {
  .basicsize = sizeof(DType)
  // ...
};

// inicialization:
HPyType_SpecParam p1[] = { { HPyType_SpecParam_Base, ctx->h_TypeType }, 0 };
HPy dtype_meta = HPyType_FromSpec(ctx, DTypeMeta_spec, p1);

HPyType_SpecParam p2[] = { { HPyType_SpecParam_Metaclass, metaclass }, 0 };
HPy dtype = HPyType_FromSpec(ctx, DType_spec, p2);

To recap what is going on there: DTypeMeta subclasses builtin PyHeapTypeObject, dtype is an instance of DTypeMeta, so HPy_AsStruct(ctx, dtype) should ideally give us DTypeMeta*.

With HPy pure types the PyHeapTypeObject super field would disappear. Since it should be opaque anyway, we do not have anything to embed into our DTypeMeta. We just specify that the base class is ctx->h_Type. The issue is that when in HPy we call PyType_FromSpecWithBases we need to set basicsize to sizeof(PyHeapTypeObject) + sizeof(DTypeMeta) (ignoring alignment for now) for this to work on CPython. In HPy_AsStruct when we have a PyObject* at hand that is actually PyHeapTypeObject* in order to get the right offset we need to know that it's in fact PyHeapTypeObject* (or know directly the offset to use).

The suggested solution looks good if we really fear that fishing the offset from the type would be unacceptably slow. However, I think we'll need to have AsStruct variant for all combinations of builtin type x legacy/non-legacy, so in concrete terms:

HPy_AsStruct
HPy_AsStruct_Legacy
HPyType_AsStruct
HPyType_AsStruct_Legacy
HPyLong_AsStruct
HPyLong_AsStruct_Legacy
etc.

in fact, I think that the legacy ones are not necessary, because there the user should already embed the right struct inside their struct, so we do not have to calculate any offset, but such a difference between pure and legacy types would make porting harder imo. For the helpers: one would have to specify the base type as an argument of the macro, or we'd have different macros:

HPyType_HELPERS
HPyTypeType_HELPERS?
HPyLongType_HELPERS?

For "dynamic" HPy_AsStruct variant -- is that actually useful? Because if I do not know what is coming from HPy_AsStruct, I cannot make any sensible use of it? If I do not know what type of object my handle points to, I need to do some type check and after that I know that it's, for example, some subclass of Long, and so I know I need to use HPyLong_AsStruct.

---

Completely different thing is inheritance of custom user defined types (and their structs). I believe that inheritance of custom types must be actually treated differently from builtin types, because builtin types must stay opaque, while custom types should be accessible to be actually useful. There I'd agree with what we discussed on the last call, that the user should actually embed structs, example:

struct {
   int data;
} MyBaseType;

static HPyType_Spec MyBaseType_spec = {
  .basicsize = sizeof(MyBaseType)
  // ...
};

struct {
   MyBaseType base;
   int other_data;
} MyType;

static HPyType_Spec MyType_spec = {
  .basicsize = sizeof(MyType)
  // ...
};

// init:
HPy base = HPyType_FromSpec(ctx, MyBaseType_spec, { 0 });

HPyType_SpecParam p2[] = { { HPyType_SpecParam_Base, base }, 0 };
HPy my_type = HPyType_FromSpec(ctx, MyType_spec, p2);

here we will need to distinguish that in this case basicsize of MyType already contains the embedded MyBaseType and we should not calculate the basicsize to be passed to PyType_FromSpecWithBases as basicsize of MyBaseType plus our basicsize. This will make the implementation bit cumbersome, but it sounds like it could be implemented fine?

Last situation that comes to my mind is inheriting from a type from another extension, which is however not exposed via some shared header file or such, so its struct cannot be embedded. In that case I would say that setting basicsize to anything else than 0, i.e., requesting some memory for a custom struct, should be just prohibited. It seems that this is something that couldn't work before with either static or heap types on CPython, anyway?

[steve-s]

This is a relevant thread that proposes similar APIs for C API: https://mail.python.org/archives/list/[email protected]/thread/SIP3VP7JU4OBWP62KBOYGOYCVIOTXEFH/

[encukou]

FWIW, turning that proposal into s PEP is now almost on top of my CPython TODO list (after python/cpython#95992 and a bit of non-CPython work)

[timfel]

Discussion at sprint: simplify this to not dynamically calculate the payload, but instead just solve the issue specifically for PyHeapTypeObject_HEAD and thus statically. /cc @fangerer

In a second PR, we will add support to inherit from other HPy types - the offset is still PyObject_HEAD and the user embeds the struct of the other type themselves (like they do now in CPython API now). (We have to write tests for that and see if it already mostly works)

For inheriting from other builtin types, we need just an enum to specify the subset of builtin types you can inherit from and the specific macros. We need to grep the top packages to see which ones are needed first.

[encukou]

I've proposed PEP 697. Could you please check it out, and comment on Discourse?

[fangerer]

I've proposed PEP 697. Could you please check it out, and comment on Discourse?

Will do so.