vstinner commented 7 months ago

Feature or enhancement

Currently, the C API implements many constants as macros. Examples:

#define Py_False _PyObject_CAST(&_Py_FalseStruct)
#define Py_None (&_Py_NoneStruct)

I propose to export important constants as symbols in the stable ABI and the limited C API, but still implement them as macros in the non-limited C API.

Linked PRs

gh-115755
gh-116572
gh-116883
gh-125194
gh-125195

vstinner commented 7 months ago

cc @encukou @davidhewitt @da-woods who might be interested by the topic.

vstinner commented 7 months ago

I also tried a different approach, use static const:

diff --git a/Include/boolobject.h b/Include/boolobject.h
index 19aef5b1b8..a841bf1587 100644
--- a/Include/boolobject.h
+++ b/Include/boolobject.h
@@ -18,8 +18,8 @@ PyAPI_DATA(PyLongObject) _Py_FalseStruct;
 PyAPI_DATA(PyLongObject) _Py_TrueStruct;

 /* Use these macros */
-#define Py_False _PyObject_CAST(&_Py_FalseStruct)
-#define Py_True _PyObject_CAST(&_Py_TrueStruct)
+static PyObject * const Py_False = _PyObject_CAST(&_Py_FalseStruct);
+static PyObject * const Py_True = _PyObject_CAST(&_Py_TrueStruct);

 // Test if an object is the True singleton, the same as "x is True" in Python.
 PyAPI_FUNC(int) Py_IsTrue(PyObject *x);

But it changes the constant type (make them constant in the limited C API), and it doesn't work with dlopen/dlsym.

cc @pitrou

vstinner commented 7 months ago

I'm not sure how to categorize "important" vs "non-important" constants.

Maybe only "pointers" to variables are interesting?

Example of constants:

#define PY_VECTORCALL_ARGUMENTS_OFFSET \
    (_Py_STATIC_CAST(size_t, 1) << (8 * sizeof(size_t) - 1))

#define Py_single_input 256   // compile.h

#define _PyDateTime_DATE_DATASIZE 4
#define PyDateTime_CAPSULE_NAME "datetime.datetime_CAPI"

#define Py_T_DOUBLE    4

#define S_IFMT 0170000   // fileutils.h

#define METH_VARARGS  0x0001

#define SEP L'\\'

#define Py_MATH_PI 3.14159265358979323846

#define Py_NAN ((double)NAN)

In vim, I looked for :grep '\# *define [A-Za-z0-9_]\+ \+[^ \\]' Include/*.h.

zooba commented 7 months ago

Can we export them as functions rather than symbols? Symbols are the reason we've struggled so hard to get the isolation we want, because we have to invent tricky solutions that wouldn't have been necessary had they been functions.

zooba commented 7 months ago

And if functions, why not Py_GetConstant(int) and an enum to select which one is wanted? That's way more easily manageable from our POV (we'd want to document that the constant comes with an incremented refcount and is only valid in the active interpreter).

I'm not sure how to categorize "important" vs "non-important" constants.

My vote is for "important ones are PyObject *"

vstinner commented 7 months ago

Can we export them as functions rather than symbols?

Sure, it's possible. But it would be slower and require C extensions to have a different API than the non-limited C API, no?

I have a working implementation where Py_None and other constants are exported as symbols ("variables") and implemented as macros: best of both world.

zooba commented 7 months ago

Well no, they could all use the same API if they want, it'd just be the function one ;-)

And the stable ABI can be Py_GetConstant(PY_NONE) while the limited API includes #define Py_None Py_GetConstant(PY_NONE). It adds some overhead, obviously, but if we'd done it this way from the start then we wouldn't have the overhead on all the INCREF/DECREF calls instead (because we wouldn't have needed to immortalise static types and constants).

Maybe we've run out of reasons that a function would be better than a data reference? I don't see any reason to assume that, and so I think having a function would be a safer ABI.

vstinner commented 7 months ago

Symbols are the reason we've struggled so hard to get the isolation we want

Which isolation are you talking about?

I don't get which problem you're trying to solve by using a function.

define Py_None Py_GetConstant(PY_NONE)

Ah right, you would have the same API this way.

But why would these specific constants need an indirection to pass through a function call whereas all other variables (PyAPI_DATA), such as Py_Version, are directly accessible?

davidhewitt commented 7 months ago

This would be a welcome change from my side, pyo3-ffi currently does refer to _Py_NoneStruct and similar private symbols in its implementation. I think we can consume either the new symbols or a function as you prefer. If you go for functions I guess pyo3-ffi would end up writing something like the following reimplementation of the limited API macros:

#[inline]
pub unsafe fn Py_None() -> *mut PyObject {
    Py_GetConstant(Py_NONE)
}

If going for the function, would all the type object symbols be replaced too? e.g. PyBool_Type.

I have no intuition for how much the function would affect performance, it'd presumably show a big impact in microbenchmarks looking at these constants but might not have much real-world impact.

("isolation we want" in this case maybe meant subinterpreter isolation? I guess there could have been a separate None for each subinterpreter.)

zooba commented 7 months ago

Yes, subinterpreter isolation could've had separate None, which meant we wouldn't have had to synchronise refcounts between each subinterpreter.[^1] The same applies for every single static type and static singleton we have. It would also mean we could store values in our thread state structure, since we have access to that (via thread-local storage) in a function.

But also, exporting data from a dynamic library is just really limited. There's no type information, there's no compatibility options - you basically just get a pointer, and I think technically it's still a function pointer even if you pretend it's a data pointer (which matters in some memory models). So say for example that we switched to tagged Booleans - how do you "export" a pointer to 0x01 | PY_BOOLEAN_TAG without upsetting every single C compiler (let alone compilers that actually care about safety 😉 )? Through a function, it's trivial to make this change without impacting any callers.

I'm pretty sure we have a C API WG discussion topic somewhere about avoiding new data exports. And I suspect @encukou may have better reasons than I do to avoid them - he's more knowledgeable about this kind of low-level weirdness.

[^1]: We don't actually synchronise them, because we know it's prohibitive for performance. So instead we added a check on every single incref/decref to see whether the refcount should just be ignored. It's a neat solution, but has its own problems.

If you go for functions I guess pyo3-ffi would end up writing something like the following reimplementation of the limited API macros

I imagine you'd need to do something like this anyway even for an imported pointer reference? You still need to cast to PyObject somewhere along the line, and I doubt you can get much shorter than this anyway. It would be a fixed list of potential ids, at least for a given version.

A while back someone did some analysis and found that these constants aren't widely used anyway (that might have been Victor?). When returning Py_None was the main one - comparisons to the Bool constants are less Pythonic than PyObject_IsTrue/PyObject_Not, and similar alternatives for others. The type objects are used more often, though primarily for type checks, and for the most part duck-typing should work with the C API as it does in Python (in the stable ABI at least, when you aren't using macros).

davidhewitt commented 7 months ago

I imagine you'd need to do something like this anyway even for an imported pointer reference?

Yes, very true, at the moment what we have looks very similar to the above just with a different implementation instead of calling the hypothetical Py_GetConstant.

When returning Py_None was the main one - comparisons to the Bool constants are less Pythonic than PyObject_IsTrue/PyObject_Not, and similar alternatives for others.

This matches roughly what I might expect, I suspect that also Py_IsNone might drive a lot of access to the structure too.

The type objects are used more often, though primarily for type checks, and for the most part duck-typing should work with the C API as it does in Python (in the stable ABI at least, when you aren't using macros).

It could be worth mentioning that we probably do have a slight tendency to encourage type checking over duck-typing for the Rust side, if only because we associate APIs with the types like PyDict and then can make a few extra assumptions like that PyDict_Size will never fail once we've done that type check.

The most critical type checks are all implemented with type flags anyway, so the impact of making the type object access less efficient would be lessened at least.

encukou commented 7 months ago

I'm pretty sure we have a C API WG discussion topic somewhere about avoiding new data exports. And I suspect @encukou may have better reasons than I do to avoid them - he's more knowledgeable about this kind of low-level weirdness.

Nope! All I know about this comes from you warning me about data exports. (On Linux things work fine, but even there I don't carry dynamic linking details in my head. Sorry!) So I always went around the Chesterton's fence rather than try bringing it down. That said, we currently do:

    PyAPI_DATA(PyObject) _Py_NoneStruct; /* Don't use this directly */
    #define Py_None (&_Py_NoneStruct)

and that data export works just fine. I don't see any reason why exporting Py_None would work worse than exporting _Py_NoneStruct at the linker level. (Maybe something bad happens when we need to remove some symbol from the ABI?)

At the API level, I definitely agree that we should export these using getter functions. Only then, as an optimization for C/C++, we could shadow the function with a static inline one that reaches for _Py_NoneStruct directly. PyO3 would need to re-implement this (for specific CPython versions) if they need the speed; a better way to let them do that is interesting future work.

As another alternative to functions that we can support for decades, we could export PyUnstable_None, and document that it's a process-wide immortal singleton, and it will remain so until we remove the symbol (which might happen in any 3.x.0 release).

vstinner commented 7 months ago

Yes, subinterpreter isolation could've had separate None, which meant we wouldn't have had to synchronise refcounts between each subinterpreter.

Ah. There were very long discussions in the past. I was likely the first to propose adding Py_GetNone() function and implement Py_None API as #define Py_None Py_GetNone(). But other people didn't buy my idea and now Python has immortal objects, and None is immortal: https://peps.python.org/pep-0683/

Now that None is immortal in CPython, I don't see the benefits for CPython to have to go through Py_GetNone() or Py_GetConstant(PY_NONE).

So say for example that we switched to tagged Booleans - how do you "export" a pointer to 0x01 | PY_BOOLEAN_TAG without upsetting every single C compiler (let alone compilers that actually care about safety 😉 )?

If tomorrow Python switchs to tagged pointers, IMO we will need much bigger ABI changes than Py_True. We can revisit the whole ABI with tagged pointers in mind. Right now, PyObject* is not opaque, Python still exposes many structures in the limited C API, so tagged pointers simply cannot be implemented.

Designing the ABI for an hypothetical use cases sounds like a premature optimization to me.

Note: I tried to implement tagged pointers for CPython. It wasn't faster. A PyPy developer told me that for CPython design, tagged pointers wouldn't have any performance benefits. Well, prove me that I'm wrong, that would be cool :-)

zooba commented 7 months ago

At the API level, I definitely agree that we should export these using getter functions.

For ABI stability? Or another reason?

Only then, as an optimization for C/C++, we could shadow the function with a static inline one that reaches for _Py_NoneStruct directly.

I don't think this would even need to be marked "unstable". We can handle normal stability guarantees fine, just not stable ABI guarantees (because in say 3.14 we can update the macro and regular API users still get the right object returned - if it were in the stable ABI we couldn't just do that for those users, so it should start somewhere that we can).

encukou commented 7 months ago

For ABI stability? Or another reason?

For API stability. If there's any reason we need to change things, functions let us do that.

I don't think this would even need to be marked "unstable".

Yes. By “shadow” I mean the static inline variant would have the same name as the exported symbol.

vstinner commented 7 months ago

For API stability. If there's any reason we need to change things, functions let us do that.

In 30 years, the only change that occurred to these constants is that they became immortal. In term of accessing them, the API didn't change in 30 years. Would you mind to elaborate which API change do you forecast?

zooba commented 7 months ago

Unfortunately, we get to assume that there will always be change. Assuming that things will never change is a terrible engineering practice.

Some completely hypothetical changes that would impact users via data exports but could be managed through function exports:

lazy type initialization
thread/interpreter-local types
synchronized/locked access
switch from singleton to non-singleton
always return new references
run-time library delegation (e.g. load python3.dll and have it forward to whichever python3x.dll it's using)
static analysis for reference counts
"fake" singletons[^1]
deprecation/warnings/errors
GIL enforcement

[^1]: This would be like how GetCurrentProcess() on Windows always returns -1, and then functions that take a process handle treat -1 as the current process. We could return a new value for "None" that isn't the actual None instance, but all operations treat it as None.

vstinner commented 7 months ago

The stable ABI already exports many Python objects with PyAPI_DATA(), examples:

Exceptions: PyExc_ValueError.
"Static" types: PyLong_Type.
_Py_NoneStruct static object (PyObject type, not PyObject* pointer!) used by#define Py_None (&_Py_NoneStruct)` in the limited C API.

If the issues that you listed are blocker issues, we should redesign the stable ABI to address these issues for all "variables" and "constants", not only the few singletons listed in this issue.

The risk of trying to design a perfect ABI is the risk of being stuck in the design. I agree that there are issues and the current API/ABI is not perfect. But I'm also concerned about consistency with existing "data" (variables/constants).

The private _Py_NoneStruct symbol is already part of the stable ABI. I'm only proposing to expose a public symbol instead, to avoid private symbols in the limited C API / stable ABI.

I would be interested to design an ABI compatible with all listed issues. But currently, PyObject.ob_refcnt is directly part of the stable ABI, whereas Python recently added immortal objects and then deferred reference couting (in the Free Threading build). If we want to support tagged pointers for example, we should make more changes.

The stable ABI also exports variables with PyAPI_DATA() which are not objects, examples:

PyOS_InputHook callback.
Py_FileSystemDefaultEncoding configuration option (const char *).
Py_Version constant (const unsigned long).
_Py_RefTotal variable (Py_ssize_t), used by Py_INCREF/Py_DECREF (Python 3.12 limited C API no longer access it directly).

zooba commented 7 months ago

I'm not proposing to go back and fix historical mistakes - I'm just saying we shouldn't deliberately keep making things worse.

None of those needed to be data, and all of them would've been better as functions. But they're in the stable ABI now, so we're stuck with them for however many years we care to support users. That doesn't mean we need to add more things to be stuck with.

zooba commented 7 months ago

To make it as easy as I can, the only argument that's really going to convince me here is if it's significantly easier for non-C users to use exported data compared to exported functions.

I'm not going to be convinced by perf (if you want that, use the normal API), equivalent ease, "we did it before" or "we'll never need to change it" arguments. But if users start lining up saying that "yes, dynamically loading data references from python3.dll is significantly simpler than calling a function from the same DLL for us", then I may concede that we should do it that way.

encukou commented 7 months ago

If the issues that you listed are blocker issues, we should redesign the stable ABI to address these issues for all "variables" and "constants", not only the few singletons listed in this issue.

Yes! I think we should do that in abi4. Expose only functions; and for API compatibility, wrap the function calls in macros.

The risk of trying to design a perfect ABI is the risk of being stuck in the design.

We do have to live with our mistakes, but that doesn't mean we need to repeat them.

vstinner commented 7 months ago

I created https://github.com/capi-workgroup/decisions/issues/16 to take a decision on this issue.

erlend-aasland commented 6 months ago

python / cpython

C API: Implement singletons as functions calls in the stable ABI 3.13 #115754

Feature or enhancement

Linked PRs

define Py_None Py_GetConstant(PY_NONE)

91927