Get some of the limited API working with Py_LIMITED_API defined #5550

da-woods · 2023-07-20T19:29:22Z

This gets a minimal limited API example working with Py_LIMITED_API defined, and adds a small test to prove it.

scoder · 2023-07-20T20:01:03Z

Cython/Utility/FunctionArguments.c

-            if (pos >= PyTuple_GET_SIZE(kwds)) break;
-            key = PyTuple_GET_ITEM(kwds, pos);
+            if (pos >= __Pyx_PyTuple_GET_SIZE(kwds)) break;
+            key = __Pyx_PyTuple_GET_ITEM(kwds, pos);


The GET_ITEM call should rather be guarded by SAFE_MACROS and BORROWED_REFS.

Still thinking about this. In both cases the BORROWED_REFS guard looks hard to implement, and isn't necessary for limited API support (where we allow borrowed refs by default).

So I'd rather just do SAFE_MACROS at this stage.

I've moved __Pyx_PyTuple_GET_SIZE inside SAFE_MACROS but I don't think this is the right solution so may come back and change it.

I think these should now be handled (at the cost of some complication)

scoder · 2023-07-20T20:02:02Z

Cython/Utility/FunctionArguments.c

@@ -414,7 +414,7 @@ static int __Pyx_MergeKeywords(PyObject *kwdict, PyObject *source_mapping) {
 // (because it's an old version of CPython or it's not CPython at all),
 // then the ..._FASTCALL macros simply alias ..._VARARGS

-#define __Pyx_Arg_VARARGS(args, i) PyTuple_GET_ITEM(args, i)
+#define __Pyx_Arg_VARARGS(args, i) __Pyx_PyTuple_GET_ITEM(args, i)


Same here, we shouldn't unconditionally use the macro.

Cython/Utility/ObjectHandling.c

scoder · 2023-07-20T20:08:14Z

Cython/Utility/TypeConversion.c

+#else
+        PyObject *from_bytes, *result;
+        PyObject *py_bytes = NULL, *arg_tuple = NULL, *kwds = NULL, *order_str = NULL;
+        from_bytes = PyObject_GetAttrString((PyObject*)&PyInt_Type, "from_bytes");


Ok, that looks expensive. But having it work at all is probably still better than having it fail..

This is the fallback code for "int type is bigger than long long" or "int type is bigger than long and Python doesn't define PY_LONG_LONG". So practically I doubt it gets used much.

But yeah, I expect it's expensive.

BTW, PY_LONG_LONG is now always defined in recent CPython versions. I think since back when they switched to C99 or a bit later, when they noticed that their code actually doesn't compile without HAVE_LONG_LONG any more. :)

da-woods · 2023-07-22T14:07:57Z

Closing and re-opening (there's an extra commit that github doesn't seem to have noticed...)

…th-py-limited-api

Mostly that we store the PyCFunctionObject as an attribute of it rather than "inheriting" from PyCFunctionObject.

…api-class

… into limited-api-with-py-limited-api

da-woods · 2023-08-11T18:34:45Z

Cython/Utility/ModuleSetupCode.c

+#if CYTHON_COMPILING_IN_LIMITED_API
+    // Note that the limited API doesn't know about PyCodeObject, so the type of this
+    // is PyObject (unlike for the main API)
+    static CYTHON_INLINE PyObject* __Pyx_PyCode_New(int a, int p, int k, int l, int s, int f,


I've also refactored this slightly again, since it used replace and so was also broken on Py3.7. Might be a bit more performant than before, but probably relies on somewhat unstable (Python) API

aws-taylor · 2023-08-11T18:58:36Z

Two other small ones related to the (non) existence of tp_free in PyTypeObject:

code.putln("(*Py_TYPE(o)->tp_free)(o);")

Reading through https://hg.python.org/cpython/rev/655d7a55c165, it seems like the fix is probably something along the lines of:

code.putln("((freefunc)PyType_GetSlot(Py_TYPE(o), Py_tp_free))(o);")

da-woods · 2023-08-11T19:34:08Z

Two other small ones related to the (non) existence of tp_free in PyTypeObject:

The follow up PR to this will do classes. I'd got the first of those lines. I hadn't got the second, but I'll make sure it add it - thanks

…th-py-limited-api

aws-taylor · 2023-08-11T20:06:33Z

Two other small ones related to the (non) existence of tp_free in PyTypeObject:

The follow up PR to this will do classes. I'd got the first of those lines. I hadn't got the second, but I'll make sure it add it - thanks

Awesome. Thanks again for this change; I'm really hoping this can get out the door. If it wasn't obvious, I'm working through a stack of errors for my own codebase. Let me know if you'd prefer I open tickets as I come across issues or just piggy-back here.

aws-taylor · 2023-08-11T20:12:05Z

Another 2 I came across related to tp_basicsize:

cython/Cython/Compiler/ModuleNode.py

Line 1582 in db995de

"if (CYTHON_COMPILING_IN_CPYTHON && likely((int)(%s > 0) & (int)(t->tp_basicsize == sizeof(%s))%s)) {" % (
cython/Cython/Compiler/ModuleNode.py

Line 1818 in db995de

"if (CYTHON_COMPILING_IN_CPYTHON && ((int)(%s < %d) & (int)(Py_TYPE(o)->tp_basicsize == sizeof(%s))%s)) {" % (

Unlike tp_free, there doesn't appear to be a slot macro for Py_tp_basicsize. I think this is because under the limited API, there are no built-in types, and thus there are no special types for which sizeof != tp_basicsize. If that's true, then I think the fix may be to simply ignore this clause in the if statement when compiling with the limited API, because it'll always be true. There may also be a fix involving basicsize_builtins_map.

da-woods · 2023-08-11T20:53:14Z

The PyPy failure looks to be a mess up with fused function optional arguments.

The code it's generating is

values[3] = __Pyx_Arg_NEWREF(((PyObject *)((long)((long)2))));

With the __Pyx_Arg_NEWREF being added by this PR, and being the bit that's breaking. However it's plainly wrong even without this PR - it just isn't generating invalid reference counting.

Unfortunately this pretty much blocks this PR until that's sorted I think.

aws-taylor · 2023-08-11T20:53:15Z

Another 1-liner related to tp_dictoffset:

cython/Cython/Utility/ExtensionTypes.c

Line 138 in c0870f9

if (dictoffset == 0 && b->tp_dictoffset)

Per https://docs.python.org/3/c-api/typeobj.html#c.PyTypeObject.tp_dictoffset, tp_dictoffset is supposed to be considered write-only. I think the fix might be:

if (dictoffset == 0 && PyObject_GenericGetDict(b, NULL) )

da-woods · 2023-08-11T20:58:26Z

Another 1-liner related to tp_dictoffset:

This is already fixed in the follow-up

tp_basicsize

This isn't. It probably isn't too hard - it's already conditional on CYTHON_COMPILING_IN_CPYTHON (which is off with the limited API) - the condition just needs to be moved to exclude the code, rather than being part of the if-statement

da-woods · 2023-08-11T21:46:15Z

Depends on #5614

da-woods · 2023-08-11T22:06:49Z

Cython/Utility/ModuleSetupCode.c


+    // Cython uses these constants but they are not available in the limited API.
+    // (it'd be nice if there was a more robust way of looking these up)
+    #ifndef CO_OPTIMIZED


I think these would be better read from inspect when the module is initialized (i.e. much like the debug flag) - it feels a bit against the spirit of the limited API to just redefine constants that have been intentionally excluded.

With that said, I'll create an issue when this PR is done rather than try to make too many more changes.

To fix PyPy errors

Get a some of the limited API working with Py_LIMITED_API defined

658b9dc

This gets a minimal limited API example working with Py_LIMITED_API defined, and adds a small test to prove it.

da-woods added the limited api label Jul 20, 2023

Try to fix setup.py

41807c0

scoder reviewed Jul 20, 2023

View reviewed changes

da-woods added 5 commits July 21, 2023 20:39

Small fixes

5349cd9

Move guards to "assume safe macros"

037c0f3

Make arg spliting code handle avoid borrowed references

13aab71

Reorder imports to allow monkey patching

743a2ba

Fix syntax error in regular mode

8d7da21

da-woods closed this Jul 22, 2023

da-woods reopened this Jul 22, 2023

da-woods added 8 commits July 22, 2023 15:09

Merge remote-tracking branch 'real_origin/master' into limited-api-wi…

b9cb9d6

…th-py-limited-api

More fixes

bb97acb

Fix code style

b2227ab

Fix a few more tests

335a718

Slightly improve error message

63a3963

ITEM and GetItem were the wrong way round

edc8242

Fix struct definition

0c3eba8

Changes CyFunction to compile with Py_LIMITED_API

516d5a7

Mostly that we store the PyCFunctionObject as an attribute of it rather than "inheriting" from PyCFunctionObject.

da-woods mentioned this pull request Jul 22, 2023

Change CyFunction to compile with Py_LIMITED_API #5556

Merged

da-woods added 2 commits July 22, 2023 21:30

Remove some spurious internal includes

77829d9

Switch orders

5b743a1

da-woods changed the title ~~Get a some of the limited API working with Py_LIMITED_API defined~~ Get some of the limited API working with Py_LIMITED_API defined Jul 23, 2023

da-woods and others added 6 commits July 23, 2023 08:49

Fixup

0a3163c

Merge conditions

7fdefa2

Initialize weakref

a7262dc

Fix reference counting mistake

ef209f8

Merge branch 'limited-api-cyfunc-2' into limited-api-with-py-limited-…

c9cdf46

…api-class

Minimal change to get classes working

5e2be08

da-woods mentioned this pull request Aug 11, 2023

CO_OPTIMIZED, CO_NEWLOCALS don't exist in Limited API #5611

Closed

da-woods added 3 commits August 11, 2023 18:50

Fix PyCode_New on 3.7

e98acbe

Replace replace on Python 3.7

cf5facc

Merge remote-tracking branch 'origin/limited-api-with-py-limited-api'…

9721840

… into limited-api-with-py-limited-api

da-woods commented Aug 11, 2023

View reviewed changes

da-woods added 2 commits August 11, 2023 20:37

Fix 3.11 typo

60f8a75

Merge remote-tracking branch 'real_origin/master' into limited-api-wi…

f932e43

…th-py-limited-api

Let CPython compile with assume_safe_macros

08744ed

da-woods commented Aug 11, 2023

View reviewed changes

da-woods closed this Aug 13, 2023

da-woods reopened this Aug 13, 2023

da-woods added 3 commits August 13, 2023 20:38

Fixed function argument reference counting code

c3ba32c

To fix PyPy errors

Silence warning on newref

aa0a4c5

Make sure ownership is properly transford out of ParseOptionalArguments

2c1c050

da-woods added this to the 3.0.1 milestone Aug 14, 2023

da-woods merged commit e1d9c5a into cython:master Aug 14, 2023
75 checks passed

da-woods deleted the limited-api-with-py-limited-api branch August 14, 2023 11:35

This was referenced Aug 14, 2023

[ENH] Use caching to speed up limited API exception handling code #5618

Open

[ENH] Get CO_ flags from inspect in limited API #5619

Open

da-woods restored the limited-api-with-py-limited-api branch August 14, 2023 20:48

isc-adang mentioned this pull request Aug 26, 2023

[autoupdate] Update Cython from 3.0.0 to 3.0.2 intersystems/iknow#359

Merged

da-woods mentioned this pull request Apr 8, 2024

Fix refcounting of default arguments with CYTHON_AVOID_BORROWED_REFS #6130

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Get some of the limited API working with Py_LIMITED_API defined #5550

Get some of the limited API working with Py_LIMITED_API defined #5550

da-woods commented Jul 20, 2023

scoder Jul 20, 2023

da-woods Jul 21, 2023

da-woods Jul 22, 2023

scoder Jul 20, 2023

scoder Jul 20, 2023

da-woods Jul 21, 2023

scoder Aug 11, 2023

da-woods commented Jul 22, 2023

da-woods Aug 11, 2023

aws-taylor commented Aug 11, 2023

da-woods commented Aug 11, 2023

aws-taylor commented Aug 11, 2023

aws-taylor commented Aug 11, 2023

da-woods commented Aug 11, 2023

aws-taylor commented Aug 11, 2023

da-woods commented Aug 11, 2023

da-woods commented Aug 11, 2023

da-woods Aug 11, 2023 •

edited

Get some of the limited API working with Py_LIMITED_API defined #5550

Get some of the limited API working with Py_LIMITED_API defined #5550

Conversation

da-woods commented Jul 20, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

da-woods commented Jul 22, 2023

Choose a reason for hiding this comment

aws-taylor commented Aug 11, 2023

da-woods commented Aug 11, 2023

aws-taylor commented Aug 11, 2023

aws-taylor commented Aug 11, 2023

da-woods commented Aug 11, 2023

aws-taylor commented Aug 11, 2023

da-woods commented Aug 11, 2023

da-woods commented Aug 11, 2023

da-woods Aug 11, 2023 • edited

Choose a reason for hiding this comment

da-woods Aug 11, 2023 •

edited