Skip to content

Commit 53ccefa

Browse files
committed
Remove virtual call to PythonNativeWrapper#getReplacement
1 parent c114a3e commit 53ccefa

File tree

4 files changed

+47
-48
lines changed

4 files changed

+47
-48
lines changed

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/cext/capi/PyMemoryViewWrapper.java

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -159,7 +159,6 @@ private static Object allocate(PMemoryView object) {
159159
return mem;
160160
}
161161

162-
@Override
163162
public Object getReplacement(InteropLibrary lib) {
164163
if (replacement == null) {
165164
Object pointerObject = allocate((PMemoryView) getDelegate());

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/cext/capi/PythonClassNativeWrapper.java

Lines changed: 29 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -191,37 +191,40 @@ public String toString() {
191191
return PythonUtils.formatJString("PythonClassNativeWrapper(%s, isNative=%s)", getDelegate(), isNative());
192192
}
193193

194-
@Override
195-
@TruffleBoundary
196194
public Object getReplacement(InteropLibrary lib) {
197-
if (replacement == null) {
198-
/*
199-
* Note: it's important that we first allocate the empty 'PyTypeStruct' and register it
200-
* to the wrapper before we do the type's initialization. Otherwise, we will run into an
201-
* infinite recursion because, e.g., some type uses 'None', so the 'NoneType' will be
202-
* transformed to native but 'NoneType' may have some field that is initialized with
203-
* 'None' and so on.
204-
*
205-
* If we first set the empty struct and initialize it afterward, everything is fine.
206-
*/
207-
PythonManagedClass clazz = (PythonManagedClass) getDelegate();
208-
boolean heaptype = (GetTypeFlagsNode.executeUncached(clazz) & TypeFlags.HEAPTYPE) != 0;
209-
long size = CStructs.PyTypeObject.size();
210-
if (heaptype) {
211-
size = CStructs.PyHeapTypeObject.size();
212-
if (GetClassNode.executeUncached(clazz) instanceof PythonAbstractNativeObject nativeMetatype) {
213-
// TODO should call the metatype's tp_alloc
214-
size = TypeNodes.GetBasicSizeNode.executeUncached(nativeMetatype);
215-
}
216-
}
217-
Object pointerObject = AllocateNode.allocUncached(size);
218-
replacement = registerReplacement(pointerObject, true, lib);
219-
220-
ToNativeTypeNode.initializeType(this, pointerObject, heaptype);
195+
if (CompilerDirectives.injectBranchProbability(CompilerDirectives.SLOWPATH_PROBABILITY, replacement == null)) {
196+
initializeReplacement(lib);
221197
}
222198
return replacement;
223199
}
224200

201+
@TruffleBoundary
202+
private void initializeReplacement(InteropLibrary lib) {
203+
/*
204+
* Note: it's important that we first allocate the empty 'PyTypeStruct' and register it to
205+
* the wrapper before we do the type's initialization. Otherwise, we will run into an
206+
* infinite recursion because, e.g., some type uses 'None', so the 'NoneType' will be
207+
* transformed to native but 'NoneType' may have some field that is initialized with 'None'
208+
* and so on.
209+
*
210+
* If we first set the empty struct and initialize it afterward, everything is fine.
211+
*/
212+
PythonManagedClass clazz = (PythonManagedClass) getDelegate();
213+
boolean heaptype = (GetTypeFlagsNode.executeUncached(clazz) & TypeFlags.HEAPTYPE) != 0;
214+
long size = CStructs.PyTypeObject.size();
215+
if (heaptype) {
216+
size = CStructs.PyHeapTypeObject.size();
217+
if (GetClassNode.executeUncached(clazz) instanceof PythonAbstractNativeObject nativeMetatype) {
218+
// TODO should call the metatype's tp_alloc
219+
size = TypeNodes.GetBasicSizeNode.executeUncached(nativeMetatype);
220+
}
221+
}
222+
Object pointerObject = AllocateNode.allocUncached(size);
223+
replacement = registerReplacement(pointerObject, true, lib);
224+
225+
ToNativeTypeNode.initializeType(this, pointerObject, heaptype);
226+
}
227+
225228
/**
226229
* Does not initialize the replacement.
227230
*/

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/cext/capi/PythonNativeWrapper.java

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -117,8 +117,9 @@ public final boolean isNative() {
117117
* <p>
118118
* Therefore, wrappers may return {@code true} and the appropriate <it>Python-to-native</it>
119119
* transition code will consider that and eagerly return the pointer object. If {@code true} is
120-
* returned, the wrapper must also implement {@link #getReplacement(InteropLibrary)} which
121-
* returns the pointer object. Furthermore, wrappers must use
120+
* returned, the wrapper must also implement specialization in
121+
* {@link com.oracle.graal.python.builtins.objects.cext.capi.transitions.GetReplacementNode}
122+
* which returns the pointer object. Furthermore, wrappers must use
122123
* {@link #registerReplacement(Object, boolean, InteropLibrary)} to register the allocated
123124
* native memory in order that the native pointer can be resolved to the managed wrapper in the
124125
* <it>native-to-Python</it> transition.
@@ -134,10 +135,6 @@ public final void setReplacement(Object replacement) {
134135
this.replacement = replacement;
135136
}
136137

137-
public Object getReplacement(@SuppressWarnings("unused") InteropLibrary lib) {
138-
throw CompilerDirectives.shouldNotReachHere();
139-
}
140-
141138
@TruffleBoundary
142139
public final Object registerReplacement(Object pointer, boolean freeAtCollection, InteropLibrary lib) {
143140
LOGGER.finest(() -> PythonUtils.formatJString("assigning %s with %s", getDelegate(), pointer));

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/cext/capi/transitions/GetReplacementNode.java

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2023, 2023, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2023, 2025, 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
@@ -40,9 +40,12 @@
4040
*/
4141
package com.oracle.graal.python.builtins.objects.cext.capi.transitions;
4242

43+
import com.oracle.graal.python.builtins.objects.cext.capi.PyMemoryViewWrapper;
44+
import com.oracle.graal.python.builtins.objects.cext.capi.PythonClassNativeWrapper;
4345
import com.oracle.graal.python.builtins.objects.cext.capi.PythonNativeWrapper;
4446
import com.oracle.graal.python.builtins.objects.cext.capi.PythonNativeWrapper.PythonAbstractObjectNativeWrapper;
4547
import com.oracle.graal.python.runtime.PythonContext;
48+
import com.oracle.truffle.api.CompilerDirectives;
4649
import com.oracle.truffle.api.dsl.Cached.Shared;
4750
import com.oracle.truffle.api.dsl.GenerateCached;
4851
import com.oracle.truffle.api.dsl.GenerateInline;
@@ -60,27 +63,24 @@ public abstract class GetReplacementNode extends Node {
6063
public abstract Object execute(Node inliningTarget, PythonNativeWrapper wrapper);
6164

6265
@Specialization(guards = "isReplacingWrapper(wrapper)")
63-
static Object doReplacingWrapper(PythonNativeWrapper wrapper,
66+
static Object doReplacingWrapper(PyMemoryViewWrapper wrapper,
6467
@Shared @CachedLibrary(limit = "3") InteropLibrary lib) {
6568
return wrapper.getReplacement(lib);
6669
}
6770

68-
@Specialization(guards = "!isReplacingWrapper(wrapper)")
69-
static Object doWrapper(Node inliningTarget, PythonNativeWrapper wrapper) {
70-
if (wrapper instanceof PythonAbstractObjectNativeWrapper objectNativeWrapper && !PythonContext.get(inliningTarget).isNativeAccessAllowed()) {
71-
// LLVM managed code path
72-
return objectNativeWrapper;
73-
}
74-
return null;
71+
@Specialization(guards = "isReplacingWrapper(wrapper)")
72+
static Object doReplacingWrapper(PythonClassNativeWrapper wrapper,
73+
@Shared @CachedLibrary(limit = "3") InteropLibrary lib) {
74+
return wrapper.getReplacement(lib);
7575
}
7676

77-
@Specialization(replaces = {"doReplacingWrapper", "doWrapper"})
78-
static Object doGeneric(Node inliningTarget, PythonNativeWrapper wrapper,
79-
@Shared @CachedLibrary(limit = "3") InteropLibrary lib) {
77+
@Specialization(guards = "isReplacingWrapper(wrapper)")
78+
static Object doReplacingWrapper(PythonNativeWrapper wrapper) {
79+
throw CompilerDirectives.shouldNotReachHere("Unexpected replacing wrapper");
80+
}
8081

81-
if (wrapper.isReplacingWrapper()) {
82-
return wrapper.getReplacement(lib);
83-
}
82+
@Specialization(guards = "!isReplacingWrapper(wrapper)")
83+
static Object doWrapper(Node inliningTarget, PythonNativeWrapper wrapper) {
8484
if (wrapper instanceof PythonAbstractObjectNativeWrapper objectNativeWrapper && !PythonContext.get(inliningTarget).isNativeAccessAllowed()) {
8585
// LLVM managed code path
8686
return objectNativeWrapper;

0 commit comments

Comments
 (0)