Skip to content

Commit a90d525

Browse files
msimacekmsimacek
committed
Implement reference queue for buffer releases
1 parent 8d1261a commit a90d525

File tree

7 files changed

+221
-65
lines changed

7 files changed

+221
-65
lines changed

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

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,8 @@
3838
# SOFTWARE.
3939

4040
import sys
41+
import time
42+
import gc
4143
import itertools
4244
from . import CPyExtTestCase, CPyExtFunction, CPyExtType, unhandled_error_compare_with_message, unhandled_error_compare
4345
__dir__ = __file__.rpartition("/")[0]
@@ -351,10 +353,11 @@ def test_memoryview_acquire_release(self):
351353
mv2 = memoryview(obj)
352354
mv3 = mv2[1:]
353355
assert obj.get_bufcount() == 2
354-
mv1.release()
355-
assert obj.get_bufcount() == 1
356356
assert mv2[3] == 126
357357
mv2.release()
358-
assert obj.get_bufcount() == 1
359-
mv3.release()
358+
assert mv3[2] == 126
359+
del mv1
360+
del mv3
361+
gc.collect()
362+
time.sleep(1)
360363
assert obj.get_bufcount() == 0

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

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -154,6 +154,7 @@
154154
import com.oracle.graal.python.builtins.objects.map.PMap;
155155
import com.oracle.graal.python.builtins.objects.memoryview.IntrinsifiedPMemoryView;
156156
import com.oracle.graal.python.builtins.objects.memoryview.ManagedBuffer;
157+
import com.oracle.graal.python.builtins.objects.memoryview.MemoryViewNodes;
157158
import com.oracle.graal.python.builtins.objects.memoryview.PBuffer;
158159
import com.oracle.graal.python.builtins.objects.module.PythonModule;
159160
import com.oracle.graal.python.builtins.objects.object.ObjectBuiltins;
@@ -3330,26 +3331,28 @@ public final IntrinsifiedPMemoryView create(Object object) {
33303331
// complex, because they don't have an underlying byte array
33313332
@Specialization
33323333
static IntrinsifiedPMemoryView fromBytes(@SuppressWarnings("unused") Object cls, PBytes object,
3334+
@Shared("getQueue") @Cached MemoryViewNodes.GetBufferReferences getQueue,
33333335
@Cached SequenceNodes.GetSequenceStorageNode getSequenceStorageNode,
33343336
@Cached SequenceStorageNodes.LenNode lenNode) {
33353337
SequenceStorage storage = getSequenceStorageNode.execute(object);
3336-
return fromManaged(object, 1, lenNode.execute(storage), true, "B", false);
3338+
return fromManaged(object, 1, lenNode.execute(storage), true, "B", false, getQueue.execute());
33373339
}
33383340

33393341
@Specialization
33403342
static IntrinsifiedPMemoryView fromByteArray(@SuppressWarnings("unused") Object cls, PByteArray object,
3343+
@Shared("getQueue") @Cached MemoryViewNodes.GetBufferReferences getQueue,
33413344
@Cached SequenceNodes.GetSequenceStorageNode getSequenceStorageNode,
33423345
@Cached SequenceStorageNodes.LenNode lenNode) {
33433346
SequenceStorage storage = getSequenceStorageNode.execute(object);
3344-
// TODO needsRelease=true
3345-
return fromManaged(object, 1, lenNode.execute(storage), false, "B", false);
3347+
return fromManaged(object, 1, lenNode.execute(storage), false, "B", true, getQueue.execute());
33463348
}
33473349

33483350
@Specialization
3349-
IntrinsifiedPMemoryView fromMemoryView(@SuppressWarnings("unused") Object cls, IntrinsifiedPMemoryView object) {
3351+
IntrinsifiedPMemoryView fromMemoryView(@SuppressWarnings("unused") Object cls, IntrinsifiedPMemoryView object,
3352+
@Shared("getQueue") @Cached MemoryViewNodes.GetBufferReferences getQueue) {
33503353
object.checkReleased(this);
33513354
return new IntrinsifiedPMemoryView(PythonBuiltinClassType.PMemoryView, PythonBuiltinClassType.PMemoryView.getInstanceShape(),
3352-
object.getManagedBuffer(), object.getOwner(), object.getLength(),
3355+
getQueue.execute(), object.getManagedBuffer(), object.getOwner(), object.getLength(),
33533356
object.isReadOnly(), object.getItemSize(), object.getFormatString(), object.getDimensions(),
33543357
object.getBufferPointer(), object.getOffset(), object.getBufferShape(), object.getBufferStrides(),
33553358
object.getBufferSuboffsets(), object.getFlags());
@@ -3368,15 +3371,16 @@ IntrinsifiedPMemoryView error(@SuppressWarnings("unused") Object cls, Object obj
33683371
throw raise(TypeError, ErrorMessages.MEMORYVIEW_A_BYTES_LIKE_OBJECT_REQUIRED_NOT_P, object);
33693372
}
33703373

3371-
private static IntrinsifiedPMemoryView fromManaged(Object object, int itemsize, int length, boolean readonly, String format, boolean needsRelease) {
3374+
private static IntrinsifiedPMemoryView fromManaged(Object object, int itemsize, int length, boolean readonly, String format, boolean needsRelease,
3375+
MemoryViewNodes.BufferReferences refQueue) {
33723376
// TODO factory
33733377
ManagedBuffer managedBuffer = null;
33743378
if (needsRelease) {
33753379
// TODO We should lock the underlying storage for resizing
33763380
managedBuffer = new ManagedBuffer(object);
33773381
}
33783382
return new IntrinsifiedPMemoryView(PythonBuiltinClassType.PMemoryView, PythonBuiltinClassType.PMemoryView.getInstanceShape(),
3379-
managedBuffer, object, length * itemsize, readonly, itemsize, format, 1,
3383+
refQueue, managedBuffer, object, length * itemsize, readonly, itemsize, format, 1,
33803384
null, 0, new int[]{length}, new int[]{itemsize}, null,
33813385
IntrinsifiedPMemoryView.FLAG_C | IntrinsifiedPMemoryView.FLAG_FORTRAN);
33823386
}

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1576,7 +1576,8 @@ Object wrap(VirtualFrame frame, Object bufferStructPointer, Object ownerObj, Obj
15761576
@Cached CastToJavaIntExactNode castToIntNode,
15771577
@Cached AsPythonObjectNode asPythonObjectNode,
15781578
@Cached ToNewRefNode toNewRefNode,
1579-
@Cached GetNativeNullNode getNativeNullNode) {
1579+
@Cached GetNativeNullNode getNativeNullNode,
1580+
@Cached MemoryViewNodes.GetBufferReferences getQueue) {
15801581
try {
15811582
int ndim = castToIntNode.execute(ndimObj);
15821583
int itemsize = castToIntNode.execute(itemsizeObj);
@@ -1620,9 +1621,8 @@ Object wrap(VirtualFrame frame, Object bufferStructPointer, Object ownerObj, Obj
16201621
if (!lib.isNull(releasefn)) {
16211622
managedBuffer = new ManagedBuffer(owner, bufferStructPointer, releasefn);
16221623
}
1623-
IntrinsifiedPMemoryView memoryview = new IntrinsifiedPMemoryView(PythonBuiltinClassType.PMemoryView,
1624-
PythonBuiltinClassType.PMemoryView.getInstanceShape(),
1625-
managedBuffer, owner, len, readonly, itemsize, format, ndim, bufPointer, 0, shape, strides, suboffsets, flags);
1624+
IntrinsifiedPMemoryView memoryview = new IntrinsifiedPMemoryView(PythonBuiltinClassType.PMemoryView, PythonBuiltinClassType.PMemoryView.getInstanceShape(),
1625+
getQueue.execute(), managedBuffer, owner, len, readonly, itemsize, format, ndim, bufPointer, 0, shape, strides, suboffsets, flags);
16261626
return toNewRefNode.execute(memoryview);
16271627
} catch (PException e) {
16281628
transformToNative(frame, e);

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/memoryview/BufferReference.java

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
package com.oracle.graal.python.builtins.objects.memoryview;
2+
3+
import java.lang.ref.PhantomReference;
4+
import java.lang.ref.ReferenceQueue;
5+
6+
class BufferReference extends PhantomReference<Object> {
7+
private final ManagedBuffer managedBuffer;
8+
private boolean released;
9+
10+
public BufferReference(Object referent, ManagedBuffer managedBuffer, ReferenceQueue<? super Object> q) {
11+
super(referent, q);
12+
assert managedBuffer != null;
13+
managedBuffer.incrementExports();
14+
this.managedBuffer = managedBuffer;
15+
}
16+
17+
public ManagedBuffer getManagedBuffer() {
18+
return managedBuffer;
19+
}
20+
21+
public boolean isReleased() {
22+
return released;
23+
}
24+
25+
public void markReleased() {
26+
this.released = true;
27+
}
28+
}

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/memoryview/IntrinsifiedPMemoryView.java

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@ public class IntrinsifiedPMemoryView extends PythonBuiltinObject {
2424
public static final int FLAG_SCALAR = 0x008;
2525
public static final int FLAG_PIL = 0x010;
2626

27-
private ManagedBuffer managedBuffer;
2827
private Object owner;
2928
private final int len;
3029
private final boolean readonly;
@@ -40,17 +39,16 @@ public class IntrinsifiedPMemoryView extends PythonBuiltinObject {
4039
private final int[] strides;
4140
private final int[] suboffsets;
4241

42+
// Count of exports via native buffer interface
4343
private final AtomicInteger exports = new AtomicInteger();
44+
// Phantom ref to this object that will decref/release the managed buffer if any
45+
private BufferReference reference;
4446
private int flags;
4547

46-
public IntrinsifiedPMemoryView(Object cls, Shape instanceShape, ManagedBuffer managedBuffer, Object owner,
48+
public IntrinsifiedPMemoryView(Object cls, Shape instanceShape, MemoryViewNodes.BufferReferences references, ManagedBuffer managedBuffer, Object owner,
4749
int len, boolean readonly, int itemsize, String formatString, int ndim, Object bufPointer,
4850
int offset, int[] shape, int[] strides, int[] suboffsets, int flags) {
4951
super(cls, instanceShape);
50-
this.managedBuffer = managedBuffer;
51-
if (managedBuffer != null) {
52-
managedBuffer.incrementExports();
53-
}
5452
this.owner = owner;
5553
this.len = len;
5654
this.readonly = readonly;
@@ -64,6 +62,10 @@ public IntrinsifiedPMemoryView(Object cls, Shape instanceShape, ManagedBuffer ma
6462
this.strides = strides;
6563
this.suboffsets = suboffsets;
6664
this.flags = flags;
65+
if (managedBuffer != null) {
66+
this.reference = new BufferReference(this, managedBuffer, references.queue);
67+
references.set.add(this.reference);
68+
}
6769
}
6870

6971
public enum BufferFormat {
@@ -148,7 +150,7 @@ public static int[] initStridesFromShape(int ndim, int itemsize, int[] shape) {
148150
}
149151

150152
public ManagedBuffer getManagedBuffer() {
151-
return managedBuffer;
153+
return (reference != null) ? reference.getManagedBuffer() : null;
152154
}
153155

154156
public Object getOwner() {
@@ -219,9 +221,16 @@ public AtomicInteger getExports() {
219221
return exports;
220222
}
221223

224+
public BufferReference getReference() {
225+
return reference;
226+
}
227+
222228
public void setReleased() {
223229
flags |= FLAG_RELEASED;
224-
managedBuffer = null;
230+
if (reference != null) {
231+
reference.markReleased();
232+
reference = null;
233+
}
225234
owner = null;
226235
}
227236

0 commit comments

Comments
 (0)