Skip to content

Commit 9fe3a5d

Browse files
horakivohorakivo
committed
Refactor: remove insertItem node from BasicSequenceStorage
1 parent 9a86e2a commit 9fe3a5d

File tree

13 files changed

+117
-153
lines changed

13 files changed

+117
-153
lines changed

‎graalpython/com.oracle.graal.python.test/src/com/oracle/graal/python/test/nodes/storage/SequenceStorageTests.java renamed to ‎graalpython/com.oracle.graal.python.test/src/com/oracle/graal/python/test/nodes/storage/GetItemSliceNodeTests.java

Lines changed: 7 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -1,43 +1,3 @@
1-
/*
2-
* Copyright (c) 2024, Oracle and/or its affiliates. All rights reserved.
3-
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
4-
*
5-
* The Universal Permissive License (UPL), Version 1.0
6-
*
7-
* Subject to the condition set forth below, permission is hereby granted to any
8-
* person obtaining a copy of this software, associated documentation and/or
9-
* data (collectively the "Software"), free of charge and under any and all
10-
* copyright rights in the Software, and any and all patent rights owned or
11-
* freely licensable by each licensor hereunder covering either (i) the
12-
* unmodified Software as contributed to or provided by such licensor, or (ii)
13-
* the Larger Works (as defined below), to deal in both
14-
*
15-
* (a) the Software, and
16-
*
17-
* (b) any piece of software and/or hardware listed in the lrgrwrks.txt file if
18-
* one is included with the Software each a "Larger Work" to which the Software
19-
* is contributed by such licensors),
20-
*
21-
* without restriction, including without limitation the rights to copy, create
22-
* derivative works of, display, perform, and distribute the Software and make,
23-
* use, sell, offer for sale, import, export, have made, and have sold the
24-
* Software and the Larger Work(s), and to sublicense the foregoing rights on
25-
* either these or other terms.
26-
*
27-
* This license is subject to the following condition:
28-
*
29-
* The above copyright notice and either this complete permission notice or at a
30-
* minimum a reference to the UPL must be included in all copies or substantial
31-
* portions of the Software.
32-
*
33-
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
34-
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
35-
* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
36-
* AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
37-
* LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
38-
* OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
39-
* SOFTWARE.
40-
*/
411
package com.oracle.graal.python.test.nodes.storage;
422

433
import com.oracle.graal.python.builtins.objects.common.SequenceStorageNodes.GetItemSliceNode;
@@ -58,7 +18,7 @@
5818
import static org.junit.Assert.assertFalse;
5919
import static org.junit.Assert.assertTrue;
6020

61-
public class SequenceStorageTests {
21+
public class GetItemSliceNodeTests {
6222

6323
@Before
6424
public void setUp() {
@@ -73,7 +33,7 @@ public void tearDown() {
7333
@Test
7434
public void intGetSlice() {
7535
var storage = new RootNode(null) {
76-
@Child private GetItemSliceNode getItemSliceNode = GetItemSliceNode.create();
36+
@Child @SuppressWarnings("FieldMayBeFinal") private GetItemSliceNode getItemSliceNode = GetItemSliceNode.create();
7737

7838
@Override
7939
public Object execute(VirtualFrame frame) {
@@ -92,7 +52,7 @@ public Object execute(VirtualFrame frame) {
9252
@Test
9353
public void longGetSlice() {
9454
var storage = new RootNode(null) {
95-
@Child private GetItemSliceNode getItemSliceNode = GetItemSliceNode.create();
55+
@Child @SuppressWarnings("FieldMayBeFinal") private GetItemSliceNode getItemSliceNode = GetItemSliceNode.create();
9656

9757
@Override
9858
public Object execute(VirtualFrame frame) {
@@ -111,7 +71,7 @@ public Object execute(VirtualFrame frame) {
11171
@Test
11272
public void doubleGetSlice() {
11373
var storage = new RootNode(null) {
114-
@Child private GetItemSliceNode getItemSliceNode = GetItemSliceNode.create();
74+
@Child @SuppressWarnings("FieldMayBeFinal") private GetItemSliceNode getItemSliceNode = GetItemSliceNode.create();
11575

11676
@Override
11777
public Object execute(VirtualFrame frame) {
@@ -130,7 +90,7 @@ public Object execute(VirtualFrame frame) {
13090
@Test
13191
public void byteGetSlice() {
13292
var storage = new RootNode(null) {
133-
@Child private GetItemSliceNode getItemSliceNode = GetItemSliceNode.create();
93+
@Child @SuppressWarnings("FieldMayBeFinal") private GetItemSliceNode getItemSliceNode = GetItemSliceNode.create();
13494

13595
@Override
13696
public Object execute(VirtualFrame frame) {
@@ -149,7 +109,7 @@ public Object execute(VirtualFrame frame) {
149109
@Test
150110
public void objectGetSlice() {
151111
var storage = new RootNode(null) {
152-
@Child private GetItemSliceNode getItemSliceNode = GetItemSliceNode.create();
112+
@Child @SuppressWarnings("FieldMayBeFinal") private GetItemSliceNode getItemSliceNode = GetItemSliceNode.create();
153113

154114
@Override
155115
public Object execute(VirtualFrame frame) {
@@ -168,7 +128,7 @@ public Object execute(VirtualFrame frame) {
168128
@Test
169129
public void boolGetSlice() {
170130
var storage = new RootNode(null) {
171-
@Child private GetItemSliceNode getItemSliceNode = GetItemSliceNode.create();
131+
@Child @SuppressWarnings("FieldMayBeFinal") private GetItemSliceNode getItemSliceNode = GetItemSliceNode.create();
172132

173133
@Override
174134
public Object execute(VirtualFrame frame) {

‎graalpython/com.oracle.graal.python.test/src/com/oracle/graal/python/test/nodes/storage/InsertItemNodeTest.java

Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,101 @@
1+
/*
2+
* Copyright (c) 2024, Oracle and/or its affiliates. All rights reserved.
3+
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
4+
*
5+
* The Universal Permissive License (UPL), Version 1.0
6+
*
7+
* Subject to the condition set forth below, permission is hereby granted to any
8+
* person obtaining a copy of this software, associated documentation and/or
9+
* data (collectively the "Software"), free of charge and under any and all
10+
* copyright rights in the Software, and any and all patent rights owned or
11+
* freely licensable by each licensor hereunder covering either (i) the
12+
* unmodified Software as contributed to or provided by such licensor, or (ii)
13+
* the Larger Works (as defined below), to deal in both
14+
*
15+
* (a) the Software, and
16+
*
17+
* (b) any piece of software and/or hardware listed in the lrgrwrks.txt file if
18+
* one is included with the Software each a "Larger Work" to which the Software
19+
* is contributed by such licensors),
20+
*
21+
* without restriction, including without limitation the rights to copy, create
22+
* derivative works of, display, perform, and distribute the Software and make,
23+
* use, sell, offer for sale, import, export, have made, and have sold the
24+
* Software and the Larger Work(s), and to sublicense the foregoing rights on
25+
* either these or other terms.
26+
*
27+
* This license is subject to the following condition:
28+
*
29+
* The above copyright notice and either this complete permission notice or at a
30+
* minimum a reference to the UPL must be included in all copies or substantial
31+
* portions of the Software.
32+
*
33+
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
34+
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
35+
* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
36+
* AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
37+
* LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
38+
* OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
39+
* SOFTWARE.
40+
*/
41+
package com.oracle.graal.python.test.nodes.storage;
42+
43+
import com.oracle.graal.python.builtins.objects.common.SequenceStorageNodes.InsertItemNode;
44+
import com.oracle.graal.python.runtime.sequence.storage.IntSequenceStorage;
45+
import com.oracle.graal.python.runtime.sequence.storage.ObjectSequenceStorage;
46+
import com.oracle.graal.python.test.PythonTests;
47+
import com.oracle.truffle.api.frame.VirtualFrame;
48+
import com.oracle.truffle.api.nodes.RootNode;
49+
import org.junit.After;
50+
import org.junit.Before;
51+
import org.junit.Test;
52+
53+
import static org.junit.Assert.assertEquals;
54+
55+
public class InsertItemNodeTest {
56+
57+
@Before
58+
public void setUp() {
59+
PythonTests.enterContext();
60+
}
61+
62+
@After
63+
public void tearDown() {
64+
PythonTests.closeContext();
65+
}
66+
67+
@Test
68+
public void objectInsert() {
69+
var storage = new RootNode(null) {
70+
@Override
71+
public Object execute(VirtualFrame frame) {
72+
var objectStorage = new ObjectSequenceStorage(new Object[]{5, 6});
73+
return InsertItemNode.executeUncached(objectStorage, 2, 11);
74+
}
75+
}.getCallTarget().call();
76+
77+
assertEquals(ObjectSequenceStorage.class, storage.getClass());
78+
var objectStorage = ((ObjectSequenceStorage) storage).getInternalArray();
79+
assertEquals(5, objectStorage[0]);
80+
assertEquals(6, objectStorage[1]);
81+
assertEquals(11, objectStorage[2]);
82+
}
83+
84+
@Test
85+
public void intInsert() {
86+
var storage = new RootNode(null) {
87+
@Override
88+
public Object execute(VirtualFrame frame) {
89+
var intStorage = new IntSequenceStorage(new int[]{5, 6});
90+
return InsertItemNode.executeUncached(intStorage, 2, 15);
91+
}
92+
}.getCallTarget().call();
93+
94+
assertEquals(IntSequenceStorage.class, storage.getClass());
95+
int[] intStorage = ((IntSequenceStorage) storage).getInternalIntArray();
96+
assertEquals(5, intStorage[0]);
97+
assertEquals(6, intStorage[1]);
98+
assertEquals(15, intStorage[2]);
99+
}
100+
101+
}

‎graalpython/com.oracle.graal.python.test/src/com/oracle/graal/python/test/runtime/SequenceStorageTests.java

Lines changed: 0 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -45,15 +45,6 @@ public void objectsGetAndSet() {
4545
assertEquals(10, store.getItemNormalized(5));
4646
}
4747

48-
@Test
49-
public void objectsInsert() {
50-
ObjectSequenceStorage store = new ObjectSequenceStorage(getObjectValues());
51-
store.insertItem(3, 42);
52-
assertEquals(42, store.getItemNormalized(3));
53-
assertEquals(6, store.getItemNormalized(6));
54-
assertEquals(7, store.length());
55-
}
56-
5748
/**
5849
* IntSequenceStorage tests.
5950
*/
@@ -68,13 +59,4 @@ public void intGetAndSet() throws SequenceStoreException {
6859
store.setItemNormalized(5, 10);
6960
assertEquals(10, store.getItemNormalized(5));
7061
}
71-
72-
@Test
73-
public void intInsert() throws SequenceStoreException {
74-
IntSequenceStorage store = new IntSequenceStorage(getIntValues());
75-
store.insertItem(3, 42);
76-
assertEquals(42, store.getItemNormalized(3));
77-
assertEquals(6, store.getItemNormalized(6));
78-
assertEquals(7, store.length());
79-
}
8062
}

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

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
import com.oracle.graal.python.builtins.objects.common.EconomicMapStorage;
2424
import com.oracle.graal.python.builtins.objects.common.HashingStorage;
2525
import com.oracle.graal.python.builtins.objects.common.HashingStorageNodes.HashingStorageSetItem;
26+
import com.oracle.graal.python.builtins.objects.common.SequenceStorageNodes.InsertItemArrayBasedStorageNode;
2627
import com.oracle.graal.python.builtins.objects.dict.PDict;
2728
import com.oracle.graal.python.builtins.objects.floats.FloatUtils;
2829
import com.oracle.graal.python.builtins.objects.tuple.PTuple;
@@ -161,7 +162,9 @@ private Object parseObjectUnicode(PJSONScanner scanner, String string, int start
161162
idx = nextIdx.value;
162163

163164
if (hasPairsHook) {
164-
listStorage.insertItem(listStorage.length(), factory.createTuple(PythonBuiltinClassType.PTuple, tupleInstanceShape, new Object[]{key, val}));
165+
var index = listStorage.length();
166+
var value = factory.createTuple(PythonBuiltinClassType.PTuple, tupleInstanceShape, new Object[]{key, val});
167+
listStorage = (ObjectSequenceStorage) InsertItemArrayBasedStorageNode.executeUncached(listStorage, index, value);
165168
} else {
166169
HashingStorage newStorage = HashingStorageSetItem.executeUncached(mapStorage, key, val);
167170
assert newStorage == mapStorage;
@@ -218,7 +221,7 @@ private Object parseArrayUnicode(PJSONScanner scanner, String string, int start,
218221

219222
/* read any JSON term */
220223
Object val = scanOnceUnicode(scanner, string, idx, nextIdx);
221-
storage.insertItem(storage.length(), val);
224+
storage = (ObjectSequenceStorage) InsertItemArrayBasedStorageNode.executeUncached(storage, storage.length(), val);
222225
idx = nextIdx.value;
223226

224227
/* skip whitespace between term and , */

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/common/SequenceStorageNodes.java

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3703,6 +3703,10 @@ static boolean isObjectSequenceStorage(SequenceStorage s) {
37033703
@GenerateInline(inlineByDefault = true)
37043704
public abstract static class InsertItemArrayBasedStorageNode extends Node {
37053705

3706+
public static SequenceStorage executeUncached(ArrayBasedSequenceStorage storage, int index, Object value) {
3707+
return SequenceStorageNodesFactory.InsertItemArrayBasedStorageNodeGen.getUncached().execute(null, storage, index, value);
3708+
}
3709+
37063710
protected abstract SequenceStorage execute(Node inliningTarget, ArrayBasedSequenceStorage storage, int index, Object value);
37073711

37083712
public final SequenceStorage executeCached(ArrayBasedSequenceStorage storage, int index, Object value) {

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/runtime/sequence/storage/BasicSequenceStorage.java

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,8 +34,6 @@ public abstract class BasicSequenceStorage extends SequenceStorage {
3434

3535
public abstract void setItemNormalized(int idx, Object value) throws SequenceStoreException;
3636

37-
public abstract void insertItem(int idx, Object value) throws SequenceStoreException;
38-
3937
/**
4038
* Get internal array object without copying. Note: The length must be taken from the sequence
4139
* storage object.

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/runtime/sequence/storage/BoolSequenceStorage.java

Lines changed: 0 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -102,27 +102,6 @@ public void setBoolItemNormalized(int idx, boolean value) {
102102
values[idx] = value;
103103
}
104104

105-
@Override
106-
public void insertItem(int idx, Object value) throws SequenceStoreException {
107-
if (value instanceof Boolean) {
108-
insertBoolItem(idx, (boolean) value);
109-
} else {
110-
throw new SequenceStoreException(value);
111-
}
112-
}
113-
114-
private void insertBoolItem(int idx, boolean value) {
115-
ensureCapacity(length + 1);
116-
117-
// shifting tail to the right by one slot
118-
for (int i = values.length - 1; i > idx; i--) {
119-
values[i] = values[i - 1];
120-
}
121-
122-
values[idx] = value;
123-
length++;
124-
}
125-
126105
@Override
127106
public Object getIndicativeValue() {
128107
return false;

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/runtime/sequence/storage/ByteSequenceStorage.java

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -116,17 +116,6 @@ public void setByteItemNormalized(int idx, byte value) {
116116
values[idx] = value;
117117
}
118118

119-
@Override
120-
public void insertItem(int idx, Object value) throws SequenceStoreException {
121-
if (value instanceof Byte) {
122-
insertByteItem(idx, (byte) value);
123-
} else if (value instanceof Integer) {
124-
insertByteItem(idx, ((Integer) value).byteValue());
125-
} else {
126-
throw new SequenceStoreException(value);
127-
}
128-
}
129-
130119
public void insertByteItem(int idx, byte value) {
131120
ensureCapacity(length + 1);
132121

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/runtime/sequence/storage/DoubleSequenceStorage.java

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -104,15 +104,6 @@ public void setDoubleItemNormalized(int idx, double value) {
104104
values[idx] = value;
105105
}
106106

107-
@Override
108-
public void insertItem(int idx, Object value) throws SequenceStoreException {
109-
if (value instanceof Double) {
110-
insertDoubleItem(idx, (double) value);
111-
} else {
112-
throw new SequenceStoreException(value);
113-
}
114-
}
115-
116107
public void insertDoubleItem(int idx, double value) {
117108
ensureCapacity(length + 1);
118109

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/runtime/sequence/storage/IntSequenceStorage.java

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -104,15 +104,6 @@ public void setIntItemNormalized(int idx, int value) {
104104
values[idx] = value;
105105
}
106106

107-
@Override
108-
public void insertItem(int idx, Object value) throws SequenceStoreException {
109-
if (value instanceof Integer) {
110-
insertIntItem(idx, (int) value);
111-
} else {
112-
throw new SequenceStoreException(value);
113-
}
114-
}
115-
116107
public void insertIntItem(int idx, int value) {
117108
ensureCapacity(length + 1);
118109

0 commit comments

Comments
 (0)