Skip to content

Commit 4cdda86

Browse files
fangererfangerer
committed
[GR-51698] [GR-51649] [GR-50680] Fully native PyType_IsSubtype.
PullRequest: graalpython/3173
2 parents c23dd1a + 5b3d4db commit 4cdda86

File tree

29 files changed

+518
-109
lines changed

29 files changed

+518
-109
lines changed

‎graalpython/com.oracle.graal.python.cext/include/cpython/tupleobject.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,8 @@ static inline Py_ssize_t PyTuple_GET_SIZE(PyObject *op) {
3333
# define PyTuple_GET_SIZE(op) PyTuple_GET_SIZE(_PyObject_CAST(op))
3434
#endif
3535

36-
#define PyTuple_GET_ITEM(op, index) PyTuple_GetItem(_PyObject_CAST(op), (index))
36+
PyAPI_FUNC(PyObject *) _PyTuple_GET_ITEM(PyObject *, Py_ssize_t);
37+
#define PyTuple_GET_ITEM(op, i) _PyTuple_GET_ITEM(_PyObject_CAST(op), (i))
3738

3839
/* Function *only* to be used to fill in brand new tuples */
3940
PyAPI_FUNC(void) PyTruffleTuple_SET_ITEM(PyObject *op, Py_ssize_t index, PyObject *value);

‎graalpython/com.oracle.graal.python.cext/src/capi.c

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -519,7 +519,12 @@ PyAPI_FUNC(void*) PyTruffle_Add_Offset(void* value, long offset) {
519519

520520
PyAPI_FUNC(void) PyTruffle_ObjectArrayRelease(PyObject** array, int32_t size) {
521521
for (int i = 0; i < size; i++) {
522-
Py_DECREF(array[i]);
522+
/* This needs to use 'Py_XDECREF' because we use this function to
523+
deallocate storages of tuples, lists, ..., where this is done in the
524+
appropriate 'tp_traverse' function which uses 'Py_VISIT' and this
525+
allows an element to be 'NULL'. Elements may, in particular, be
526+
'NULL' if a tuple dies before all elements are initialized. */
527+
Py_XDECREF(array[i]);
523528
}
524529
}
525530

‎graalpython/com.oracle.graal.python.cext/src/capi.h

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,14 @@
4949
#define NO_INLINE __attribute__((noinline))
5050
#endif
5151

52+
#if defined(__GNUC__) && (__GNUC__ > 2) && defined(__OPTIMIZE__)
53+
# define UNLIKELY(value) __builtin_expect((value), 0)
54+
# define LIKELY(value) __builtin_expect((value), 1)
55+
#else
56+
# define UNLIKELY(value) (value)
57+
# define LIKELY(value) (value)
58+
#endif
59+
5260
#ifdef MS_WINDOWS
5361
// define the below, otherwise windows' sdk defines complex to _complex and breaks us
5462
#define _COMPLEX_DEFINED
@@ -95,6 +103,10 @@ typedef struct {
95103
int getter_doc;
96104
} propertyobject;
97105

106+
typedef struct {
107+
PyObject_VAR_HEAD
108+
PyObject **ob_item;
109+
} GraalPyVarObject;
98110

99111
// {{start CAPI_BUILTINS}}
100112
#include "capi.gen.h"

‎graalpython/com.oracle.graal.python.cext/src/tupleobject.c

Lines changed: 60 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
/* Copyright (c) 2018, 2023, Oracle and/or its affiliates.
1+
/* Copyright (c) 2018, 2024, Oracle and/or its affiliates.
22
* Copyright (C) 1996-2022 Python Software Foundation
33
*
44
* Licensed under the PYTHON SOFTWARE FOUNDATION LICENSE VERSION 2
@@ -87,3 +87,62 @@ void PyTruffle_Tuple_Dealloc(PyTupleObject* self) {
8787
}
8888
Py_TYPE(self)->tp_free((PyObject *)self);
8989
}
90+
91+
PyObject*
92+
PyTuple_GetItem(PyObject* a, Py_ssize_t b) {
93+
#ifdef GRAALVM_PYTHON_LLVM_MANAGED
94+
return GraalPyTruffleTuple_GetItem(a, b);
95+
#else /* GRAALVM_PYTHON_LLVM_MANAGED */
96+
if (!PyTuple_Check(a)) {
97+
PyErr_BadInternalCall();
98+
return NULL;
99+
}
100+
PyObject *res;
101+
PyObject **ob_item;
102+
if (points_to_py_handle_space(a)) {
103+
const PyObject *ptr = pointer_to_stub((PyObject *) a);
104+
ob_item = ((GraalPyVarObject *) ptr)->ob_item;
105+
if (ob_item == NULL) {
106+
// native data ptr not set; do upcall
107+
return GraalPyTruffleTuple_GetItem(a, b);
108+
}
109+
} else {
110+
ob_item = ((PyTupleObject *) a)->ob_item;
111+
}
112+
// do index check since directly accessing the items array
113+
if (b < 0 || b >= Py_SIZE(a)) {
114+
PyErr_SetString(PyExc_IndexError, "tuple index out of range");
115+
return NULL;
116+
}
117+
assert(ob_item != NULL);
118+
return ob_item[b];
119+
#endif /* GRAALVM_PYTHON_LLVM_MANAGED */
120+
}
121+
122+
/*
123+
* Unsafe variant of PyTuple_GetItem for implementing access macro
124+
* PyTuple_GET_ITEM.
125+
*/
126+
PyObject*
127+
_PyTuple_GET_ITEM(PyObject* a, Py_ssize_t b) {
128+
#ifdef GRAALVM_PYTHON_LLVM_MANAGED
129+
return GraalPyTruffleTuple_GetItem(a, b);
130+
#else /* GRAALVM_PYTHON_LLVM_MANAGED */
131+
PyObject *res;
132+
PyObject **ob_item;
133+
if (points_to_py_handle_space(a)) {
134+
GraalPyVarObject *ptr = (GraalPyVarObject *) pointer_to_stub(a);
135+
ob_item = ((GraalPyVarObject *) ptr)->ob_item;
136+
/* The UNLIKELY is maybe not true but the branch is costly anyway. So,
137+
if we can optimize for something, it should be the path without the
138+
upcall. */
139+
if (UNLIKELY(ob_item == NULL)) {
140+
ptr->ob_item = (ob_item = GraalPy_get_PyTupleObject_ob_item(a));
141+
}
142+
} else {
143+
ob_item = ((PyTupleObject *) a)->ob_item;
144+
}
145+
assert(ob_item != NULL);
146+
return ob_item[b];
147+
#endif /* GRAALVM_PYTHON_LLVM_MANAGED */
148+
}

‎graalpython/com.oracle.graal.python.cext/src/typeobject.c

Lines changed: 44 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1524,6 +1524,45 @@ PyType_GetModuleState(PyTypeObject *type)
15241524
return _PyModule_GetState(m);
15251525
}
15261526

1527+
/* type test with subclassing support */
1528+
1529+
#ifndef GRAALVM_PYTHON_LLVM_MANAGED
1530+
static inline int
1531+
type_is_subtype_base_chain(PyTypeObject *a, PyTypeObject *b)
1532+
{
1533+
do {
1534+
if (a == b)
1535+
return 1;
1536+
a = a->tp_base;
1537+
} while (a != NULL);
1538+
1539+
return (b == &PyBaseObject_Type);
1540+
}
1541+
1542+
static inline int
1543+
type_is_subtype_mro(PyTypeObject *a, PyTypeObject *b)
1544+
{
1545+
PyObject *mro;
1546+
1547+
mro = a->tp_mro;
1548+
if (mro != NULL) {
1549+
/* Deal with multiple inheritance without recursion
1550+
by walking the MRO tuple */
1551+
Py_ssize_t i, n;
1552+
assert(PyTuple_Check(mro));
1553+
n = PyTuple_GET_SIZE(mro);
1554+
for (i = 0; i < n; i++) {
1555+
if (PyTuple_GET_ITEM(mro, i) == (PyObject *)b)
1556+
return 1;
1557+
}
1558+
return 0;
1559+
}
1560+
else
1561+
/* a is not completely initialized yet; follow tp_base */
1562+
return type_is_subtype_base_chain(a, b);
1563+
}
1564+
#endif /* GRAALVM_PYTHON_LLVM_MANAGED */
1565+
15271566
int
15281567
PyType_IsSubtype(PyTypeObject* a, PyTypeObject* b)
15291568
{
@@ -1543,10 +1582,14 @@ PyType_IsSubtype(PyTypeObject* a, PyTypeObject* b)
15431582
return PyType_FastSubclass(a, Py_TPFLAGS_UNICODE_SUBCLASS);
15441583
} else if (b == &PyDict_Type) {
15451584
return PyType_FastSubclass(a, Py_TPFLAGS_DICT_SUBCLASS);
1546-
} else if (b == PyExc_BaseException) {
1585+
} else if ((PyObject *)b == PyExc_BaseException) {
15471586
return PyType_FastSubclass(a, Py_TPFLAGS_BASE_EXC_SUBCLASS);
15481587
} else if (is_builtin_type(a) && !is_builtin_type(b)) {
15491588
return 0;
15501589
}
1590+
#ifdef GRAALVM_PYTHON_LLVM_MANAGED
15511591
return GraalPyTruffleType_IsSubtype(a, b);
1592+
#else /* GRAALVM_PYTHON_LLVM_MANAGED */
1593+
return type_is_subtype_mro(a, b);
1594+
#endif /* GRAALVM_PYTHON_LLVM_MANAGED */
15521595
}

‎graalpython/com.oracle.graal.python.test/src/tests/cpyext/test_functions.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,9 @@ def is_buffer(x):
112112
lambda args: 1 if isinstance(*args) else 0,
113113
lambda: (
114114
(1, int),
115+
("hello", str),
116+
(True, bool),
117+
(True, int),
115118
),
116119
argspec="OO",
117120
arguments=["PyObject* op", "PyTypeObject* type"],

‎graalpython/com.oracle.graal.python.test/src/tests/cpyext/test_object.py

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1318,6 +1318,18 @@ def test_take_ownership(self):
13181318
return set_value(self, value);
13191319
}
13201320
1321+
static PyObject* from_args_tuple(PyObject* self, PyObject *args) {
1322+
PyObject *result;
1323+
if (Py_SIZE(args) > 0) {
1324+
result = PyTuple_GET_ITEM(args, 0);
1325+
} else {
1326+
result = Py_None;
1327+
}
1328+
Py_INCREF(result);
1329+
(void) set_value(self, args);
1330+
return result;
1331+
}
1332+
13211333
static PyObject* get_value(PyObject* self) {
13221334
ValueObject *value_obj = (ValueObject *) self;
13231335
PyObject *res = value_obj->value;
@@ -1349,6 +1361,7 @@ def test_take_ownership(self):
13491361
tp_methods='''
13501362
{"set_value", (PyCFunction)set_value, METH_O, NULL},
13511363
{"from_tuple", (PyCFunction)from_tuple, METH_O, NULL},
1364+
{"from_args_tuple", (PyCFunction)from_args_tuple, METH_VARARGS, NULL},
13521365
{"get_value", (PyCFunction)get_value, METH_NOARGS, NULL},
13531366
{"clear_value", (PyCFunction)clear_value, METH_NOARGS, NULL},
13541367
{"own_a_lot", (PyCFunction)own_a_lot, METH_O, NULL}
@@ -1388,6 +1401,16 @@ def test_take_ownership(self):
13881401
dummy = object()
13891402
obj.own_a_lot(dummy)
13901403

1404+
obj = ValueType()
1405+
dummy = object()
1406+
obj.from_args_tuple(1, 2, 3)
1407+
# do it a second time; this time we will eagerly create a native storage
1408+
obj.from_args_tuple(1, 2, 3, "hello", "world", dummy)
1409+
value = obj.get_value()
1410+
assert value == (1, 2, 3, "hello", "world", dummy)
1411+
obj.clear_value()
1412+
assert value == (1, 2, 3, "hello", "world", dummy)
1413+
13911414
def test_async_slots(self):
13921415
import asyncio, types, functools
13931416
TestTpAsync = CPyExtType("TestTpAsync",

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/modules/cext/PythonCextTupleBuiltins.java

Lines changed: 21 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@
4242

4343
import static com.oracle.graal.python.builtins.PythonBuiltinClassType.IndexError;
4444
import static com.oracle.graal.python.builtins.modules.cext.PythonCextBuiltins.CApiCallPath.Direct;
45+
import static com.oracle.graal.python.builtins.modules.cext.PythonCextBuiltins.CApiCallPath.Ignored;
4546
import static com.oracle.graal.python.builtins.objects.cext.capi.transitions.ArgDescriptor.Int;
4647
import static com.oracle.graal.python.builtins.objects.cext.capi.transitions.ArgDescriptor.PyObject;
4748
import static com.oracle.graal.python.builtins.objects.cext.capi.transitions.ArgDescriptor.PyObjectBorrowed;
@@ -63,6 +64,7 @@
6364
import com.oracle.graal.python.builtins.objects.common.SequenceStorageNodes.ListGeneralizationNode;
6465
import com.oracle.graal.python.builtins.objects.common.SequenceStorageNodes.SetItemNode;
6566
import com.oracle.graal.python.builtins.objects.common.SequenceStorageNodes.SetItemScalarNode;
67+
import com.oracle.graal.python.builtins.objects.ints.PInt;
6668
import com.oracle.graal.python.builtins.objects.tuple.PTuple;
6769
import com.oracle.graal.python.lib.PySliceNew;
6870
import com.oracle.graal.python.lib.PyTupleSizeNode;
@@ -72,6 +74,7 @@
7274
import com.oracle.graal.python.runtime.object.PythonObjectFactory;
7375
import com.oracle.graal.python.runtime.sequence.storage.NativeObjectSequenceStorage;
7476
import com.oracle.graal.python.runtime.sequence.storage.SequenceStorage;
77+
import com.oracle.graal.python.util.PythonUtils;
7578
import com.oracle.truffle.api.dsl.Bind;
7679
import com.oracle.truffle.api.dsl.Cached;
7780
import com.oracle.truffle.api.dsl.Cached.Exclusive;
@@ -89,8 +92,21 @@ abstract static class PyTuple_New extends CApiUnaryBuiltinNode {
8992

9093
@Specialization
9194
static PTuple doGeneric(long size,
92-
@Cached PythonObjectFactory factory) {
93-
return factory.createTuple(new Object[(int) size]);
95+
@Bind("this") Node inliningTarget,
96+
@Cached PythonObjectFactory factory,
97+
@Cached PRaiseNode.Lazy raiseNode) {
98+
if (!PInt.isIntRange(size)) {
99+
throw raiseNode.get(inliningTarget).raise(PythonBuiltinClassType.MemoryError);
100+
}
101+
Object[] data = new Object[(int) size];
102+
/*
103+
* We need to fill the empty object array with 'PNone.NO_VALUE' because it may be that
104+
* the tuple is accessed with 'PyTuple_GET_ITEM' before all elements are initialized and
105+
* the corresponding storage-to-native transition would then fail because of the Java
106+
* nulls.
107+
*/
108+
PythonUtils.fill(data, 0, data.length, PNone.NO_VALUE);
109+
return factory.createTuple(data);
94110
}
95111
}
96112

@@ -192,8 +208,8 @@ private static void checkBounds(Node inliningTarget, SequenceStorage sequenceSto
192208
}
193209
}
194210

195-
@CApiBuiltin(ret = PyObjectBorrowed, args = {PyObject, Py_ssize_t}, call = Direct)
196-
public abstract static class PyTuple_GetItem extends CApiBinaryBuiltinNode {
211+
@CApiBuiltin(ret = PyObjectBorrowed, args = {PyObject, Py_ssize_t}, call = Ignored)
212+
public abstract static class PyTruffleTuple_GetItem extends CApiBinaryBuiltinNode {
197213

198214
public abstract Object execute(PTuple tuple, long key);
199215

@@ -208,21 +224,14 @@ static Object doPTuple(PTuple tuple, long key,
208224
SequenceStorage sequenceStorage = tuple.getSequenceStorage();
209225
int index = checkIndex(inliningTarget, key, sequenceStorage, raiseNode);
210226
Object result = getItemNode.execute(inliningTarget, sequenceStorage, index);
227+
assert result != null : "tuple must not contain Java null";
211228
Object promotedValue = promoteNode.execute(inliningTarget, result);
212229
if (promotedValue != null) {
213230
sequenceStorage = generalizationNode.execute(inliningTarget, sequenceStorage, promotedValue);
214231
tuple.setSequenceStorage(sequenceStorage);
215232
setItemNode.execute(inliningTarget, sequenceStorage, index, promotedValue);
216233
return promotedValue;
217234
}
218-
if (result == null) {
219-
/*
220-
* This can happen when the tuple is not fully initialized. Such tuple is not valid,
221-
* but the C code sometimes uses PyTuple_GET_ITEM to Py_XDECREF the items on error
222-
* paths when they failed to populate the tuple.
223-
*/
224-
return getNativeNull(inliningTarget);
225-
}
226235
return result;
227236
}
228237

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/modules/ctypes/memory/PointerNodes.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2023, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2023, 2024, Oracle and/or its affiliates. All rights reserved.
33
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
44
*
55
* The Universal Permissive License (UPL), Version 1.0
@@ -662,7 +662,7 @@ static long doPointerArray(Node inliningTarget, MemoryBlock memory, PointerArray
662662
@Specialization
663663
static long doMemoryView(Node inliningTarget, MemoryBlock memory, MemoryViewStorage storage, int offset,
664664
@CachedLibrary(limit = "1") InteropLibrary ilib,
665-
@Cached(inline = false) SequenceStorageNodes.StorageToNativeNode storageToNativeNode) {
665+
@Cached SequenceStorageNodes.StorageToNativeNode storageToNativeNode) {
666666
PMemoryView mv = storage.memoryView;
667667
Object ptr = null;
668668
if (mv.getBufferPointer() != null) {
@@ -672,7 +672,7 @@ static long doMemoryView(Node inliningTarget, MemoryBlock memory, MemoryViewStor
672672
ptr = nativeSequenceStorage.getPtr();
673673
}
674674
if (bytes.getSequenceStorage() instanceof ByteSequenceStorage byteSequenceStorage) {
675-
NativeSequenceStorage nativeStorage = storageToNativeNode.execute(byteSequenceStorage.getInternalByteArray(), byteSequenceStorage.length());
675+
NativeSequenceStorage nativeStorage = storageToNativeNode.execute(inliningTarget, byteSequenceStorage.getInternalByteArray(), byteSequenceStorage.length());
676676
bytes.setSequenceStorage(nativeStorage);
677677
ptr = nativeStorage.getPtr();
678678
}

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/array/ArrayNodes.java

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2020, 2023, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2020, 2024, Oracle and/or its affiliates. All rights reserved.
33
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
44
*
55
* The Universal Permissive License (UPL), Version 1.0
@@ -188,12 +188,12 @@ public abstract static class EnsureNativeStorageNode extends Node {
188188
public abstract NativeByteSequenceStorage execute(Node inliningTarget, PArray array);
189189

190190
@Specialization
191-
NativeByteSequenceStorage toNative(PArray array,
192-
@Cached(inline = false) SequenceStorageNodes.StorageToNativeNode storageToNativeNode) {
191+
static NativeByteSequenceStorage toNative(Node inliningTarget, PArray array,
192+
@Cached SequenceStorageNodes.StorageToNativeNode storageToNativeNode) {
193193
if (array.getSequenceStorage() instanceof NativeByteSequenceStorage storage) {
194194
return storage;
195195
} else if (array.getSequenceStorage() instanceof ByteSequenceStorage storage) {
196-
NativeByteSequenceStorage nativeStorage = (NativeByteSequenceStorage) storageToNativeNode.execute(storage.getInternalByteArray(), storage.length());
196+
NativeByteSequenceStorage nativeStorage = storageToNativeNode.executeBytes(inliningTarget, storage.getInternalByteArray(), storage.length());
197197
array.setSequenceStorage(nativeStorage);
198198
return nativeStorage;
199199
} else {

0 commit comments

Comments
 (0)