Skip to content

Commit cda20f8

Browse files
phoeinxphoeinx
committed
Refactor according to PR review.
Refactorings include: - Make CSVModuleBuiltins>>fieldLimit an instance variable to avoid sharing it across Python contexts. - Use `PRaiseNode.getUncached()` behind TruffleBoundary.
1 parent 78bf0d3 commit cda20f8

File tree

7 files changed

+39
-34
lines changed

7 files changed

+39
-34
lines changed

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

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -93,11 +93,12 @@
9393
@CoreFunctions(defineModule = "_csv")
9494
public final class CSVModuleBuiltins extends PythonBuiltins {
9595

96-
static long fieldLimit = 128 * 1024; // max parsed field size
9796
static final String WRITE = "write";
9897
static final String NOT_SET = "NOT_SET";
9998
static final int NOT_SET_CODEPOINT = -1;
10099

100+
long fieldLimit = 128 * 1024; // max parsed field size
101+
101102
@Override
102103
protected List<? extends NodeFactory<? extends PythonBuiltinBaseNode>> getNodeFactories() {
103104
return CSVModuleBuiltinsFactory.getFactories();
@@ -260,27 +261,27 @@ Object createReader(VirtualFrame frame, Object outputFile, Object dialectObj, PK
260261
}
261262
}
262263

263-
@Builtin(name = "field_size_limit", parameterNames = {"limit"}, doc = "Sets an upper limit on parsed fields.\n" +
264+
@Builtin(name = "field_size_limit", parameterNames = {"$mod", "limit"}, declaresExplicitSelf = true, doc = "Sets an upper limit on parsed fields.\n" +
264265
"csv.field_size_limit([limit])\n\n" +
265266
"Returns old limit. If limit is not given, no new limit is set and\n" +
266267
"the old limit is returned")
267268
@GenerateNodeFactory
268269
public abstract static class CSVFieldSizeLimitNode extends PythonBuiltinNode {
270+
269271
@Specialization
270-
long getOrSetFieldSizeLimit(VirtualFrame frame, Object newLimit,
272+
long getOrSetFieldSizeLimit(VirtualFrame frame, PythonModule self, Object newLimit,
271273
@Cached PyLongCheckExactNode checkLongNode,
272274
@Cached PyLongAsLongNode castToLong) {
273275

274-
long oldLimit = fieldLimit;
276+
CSVModuleBuiltins csvModuleBuiltins = (CSVModuleBuiltins) self.getBuiltins();
277+
long oldLimit = csvModuleBuiltins.fieldLimit;
275278

276279
if (newLimit != PNone.NO_VALUE) {
277280
if (!checkLongNode.execute(newLimit)) {
278281
throw raise(PythonBuiltinClassType.TypeError, ErrorMessages.MUST_BE_INTEGER, "limit");
279282
}
280-
281-
fieldLimit = castToLong.execute(frame, newLimit);
283+
csvModuleBuiltins.fieldLimit = castToLong.execute(frame, newLimit);
282284
}
283-
284285
return oldLimit;
285286
}
286287
}
@@ -462,7 +463,7 @@ Object doGeneric(VirtualFrame frame, PythonBuiltinClassType cls, Object dialectO
462463
getClassNode, castToJavaStringNode, isTrueNode, pyLongCheckExactNode, pyLongAsIntNode);
463464
}
464465

465-
protected boolean isCSVDialect(Object dialect) {
466+
protected static boolean isCSVDialect(Object dialect) {
466467
return dialect instanceof CSVDialect;
467468
}
468469

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

Lines changed: 21 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -63,10 +63,12 @@
6363
import com.oracle.graal.python.nodes.object.IsBuiltinClassProfile;
6464
import com.oracle.graal.python.nodes.util.CannotCastException;
6565
import com.oracle.graal.python.nodes.util.CastToJavaStringNode;
66+
import com.oracle.graal.python.runtime.PythonContext;
6667
import com.oracle.graal.python.runtime.exception.PException;
6768
import com.oracle.graal.python.runtime.object.PythonObjectFactory;
6869
import com.oracle.truffle.api.CompilerDirectives.TruffleBoundary;
6970
import com.oracle.truffle.api.exception.AbstractTruffleException;
71+
import com.oracle.truffle.api.nodes.Node;
7072
import com.oracle.truffle.api.object.Shape;
7173

7274
public final class CSVReader extends PythonBuiltinObject {
@@ -121,7 +123,7 @@ void parseSaveField() {
121123
}
122124

123125
@TruffleBoundary
124-
Object parseIterableInput() {
126+
Object parseIterableInput(Node node) {
125127
do {
126128
Object lineObj;
127129
try {
@@ -160,7 +162,7 @@ Object parseIterableInput() {
160162
}
161163

162164
this.lineNum++;
163-
this.parseLine(line);
165+
this.parseLine(node, line);
164166

165167
} while (this.state != START_RECORD);
166168

@@ -170,7 +172,7 @@ Object parseIterableInput() {
170172
return PythonObjectFactory.getUncached().createList(fields.toArray());
171173
}
172174

173-
void parseLine(String line) {
175+
void parseLine(Node node, String line) {
174176
final int lineLength = line.length();
175177

176178
/*
@@ -180,16 +182,16 @@ void parseLine(String line) {
180182
for (int offset = 0; offset < lineLength;) {
181183
final int codepoint = line.codePointAt(offset);
182184

183-
parseProcessCodePoint(codepoint);
185+
parseProcessCodePoint(node, codepoint);
184186

185187
offset += Character.charCount(codepoint);
186188
}
187189

188-
parseProcessCodePoint(EOL);
190+
parseProcessCodePoint(node, EOL);
189191
}
190192

191193
@SuppressWarnings("fallthrough")
192-
void parseProcessCodePoint(int codePoint) {
194+
void parseProcessCodePoint(Node node, int codePoint) {
193195
CSVDialect dialect = this.dialect;
194196

195197
switch (this.state) {
@@ -230,21 +232,21 @@ void parseProcessCodePoint(int codePoint) {
230232
if (dialect.quoting == QUOTE_NONNUMERIC) {
231233
this.numericField = true;
232234
}
233-
parseAddCodePoint(codePoint);
235+
parseAddCodePoint(node, codePoint);
234236
this.state = IN_FIELD;
235237
}
236238
break;
237239

238240
case ESCAPED_CHAR:
239241
if (codePoint == NEWLINE_CODEPOINT || codePoint == CARRIAGE_RETURN_CODEPOINT) {
240-
parseAddCodePoint(codePoint);
242+
parseAddCodePoint(node, codePoint);
241243
this.state = AFTER_ESCAPED_CRNL;
242244
break;
243245
}
244246
if (codePoint == EOL) {
245247
codePoint = NEWLINE_CODEPOINT;
246248
}
247-
parseAddCodePoint(codePoint);
249+
parseAddCodePoint(node, codePoint);
248250

249251
this.state = IN_FIELD;
250252
break;
@@ -271,7 +273,7 @@ void parseProcessCodePoint(int codePoint) {
271273
this.state = START_FIELD;
272274
} else {
273275
/* normal character - save in field */
274-
parseAddCodePoint(codePoint);
276+
parseAddCodePoint(node, codePoint);
275277
}
276278
break;
277279

@@ -293,15 +295,15 @@ void parseProcessCodePoint(int codePoint) {
293295
}
294296
} else {
295297
/* normal character - save in field */
296-
parseAddCodePoint(codePoint);
298+
parseAddCodePoint(node, codePoint);
297299
}
298300
break;
299301

300302
case ESCAPE_IN_QUOTED_FIELD:
301303
if (codePoint == EOL) {
302304
codePoint = NEWLINE_CODEPOINT;
303305
}
304-
parseAddCodePoint(codePoint);
306+
parseAddCodePoint(node, codePoint);
305307
this.state = IN_QUOTED_FIELD;
306308
break;
307309

@@ -310,7 +312,7 @@ void parseProcessCodePoint(int codePoint) {
310312
if (dialect.quoting != QUOTE_NONE &&
311313
codePoint == dialect.quoteCharCodePoint) {
312314
/* save "" as " */
313-
parseAddCodePoint(codePoint);
315+
parseAddCodePoint(node, codePoint);
314316
this.state = IN_QUOTED_FIELD;
315317
} else if (codePoint == dialect.delimiterCodePoint) {
316318
/* save field - wait for new field */
@@ -321,7 +323,7 @@ void parseProcessCodePoint(int codePoint) {
321323
parseSaveField();
322324
this.state = (codePoint == EOL) ? START_RECORD : EAT_CRNL;
323325
} else if (!dialect.strict) {
324-
parseAddCodePoint(codePoint);
326+
parseAddCodePoint(node, codePoint);
325327
this.state = IN_FIELD;
326328
} else {
327329
/* illegal */
@@ -344,11 +346,13 @@ void parseProcessCodePoint(int codePoint) {
344346

345347
}
346348

347-
void parseAddCodePoint(int codePoint) {
349+
void parseAddCodePoint(Node node, int codePoint) {
348350

349-
if (this.field.length() + Character.charCount(codePoint) > CSVModuleBuiltins.fieldLimit) {
351+
CSVModuleBuiltins csvModuleBuiltins = (CSVModuleBuiltins) PythonContext.get(node).lookupBuiltinModule("_csv").getBuiltins();
352+
353+
if (this.field.length() + Character.charCount(codePoint) > csvModuleBuiltins.fieldLimit) {
350354
throw PRaiseNode.getUncached().raise(PythonBuiltinClassType.CSVError, ErrorMessages.LARGER_THAN_FIELD_SIZE_LIMIT,
351-
CSVModuleBuiltins.fieldLimit);
355+
csvModuleBuiltins.fieldLimit);
352356
}
353357

354358
this.field.appendCodePoint(codePoint);

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ Object nextPos(VirtualFrame frame, CSVReader self) {
8787
Object state = IndirectCallContext.enter(frame, language, getContext(), this);
8888

8989
try {
90-
return self.parseIterableInput();
90+
return self.parseIterableInput(this);
9191
} finally {
9292
IndirectCallContext.exit(frame, language, getContext(), state);
9393
}

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@
5454
import com.oracle.graal.python.nodes.object.IsBuiltinClassProfile;
5555
import com.oracle.graal.python.runtime.exception.PException;
5656
import com.oracle.truffle.api.CompilerDirectives.TruffleBoundary;
57+
import com.oracle.truffle.api.nodes.Node;
5758
import com.oracle.truffle.api.object.Shape;
5859

5960
public final class CSVWriter extends PythonBuiltinObject {
@@ -74,7 +75,7 @@ void joinReset() {
7475
}
7576

7677
@TruffleBoundary
77-
void joinFields(Object iter) {
78+
void joinFields(Node node, Object iter) {
7879
Object field;
7980

8081
this.joinReset();
@@ -91,7 +92,7 @@ void joinFields(Object iter) {
9192

9293
if (this.numFields > 0 && this.rec.length() == 0) {
9394
if (this.dialect.quoting == QUOTE_NONE) {
94-
throw PRaiseNode.getUncached().raise(PythonBuiltinClassType.CSVError, ErrorMessages.EMPTY_FIELD_RECORD_MUST_BE_QUOTED);
95+
throw PRaiseNode.raiseUncached(node, PythonBuiltinClassType.CSVError, ErrorMessages.EMPTY_FIELD_RECORD_MUST_BE_QUOTED);
9596
}
9697
this.numFields--;
9798
this.joinAppend(null, true);

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -93,10 +93,10 @@ Object doIt(VirtualFrame frame, CSVWriter self, Object seq,
9393
}
9494

9595
// Join all fields of passed in sequence in internal buffer.
96-
PythonLanguage language = PythonLanguage.get(this);
96+
PythonLanguage language = getLanguage();
9797
Object state = IndirectCallContext.enter(frame, language, getContext(), this);
9898
try {
99-
self.joinFields(iter);
99+
self.joinFields(this, iter);
100100
} finally {
101101
IndirectCallContext.exit(frame, language, getContext(), state);
102102
}

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/lib/PyDictSetItem.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@
6262
public abstract class PyDictSetItem extends Node {
6363
public abstract void execute(Frame frame, PDict dict, Object key, Object item);
6464

65-
// We never need a frame for reading string keys
65+
// We never need a frame for setting string keys
6666
@Specialization(limit = "3")
6767
static final void setItemWithStringKey(@SuppressWarnings("unused") PDict dict, String key, Object item,
6868
@Bind("dict.getDictStorage()") HashingStorage dictStorage,

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/lib/PyNumberCheckNode.java

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,6 @@
4848
import com.oracle.graal.python.nodes.attributes.LookupCallableSlotInMRONode;
4949
import com.oracle.graal.python.nodes.object.GetClassNode;
5050
import com.oracle.truffle.api.dsl.Cached;
51-
import com.oracle.truffle.api.dsl.Fallback;
5251
import com.oracle.truffle.api.dsl.GenerateUncached;
5352
import com.oracle.truffle.api.dsl.ImportStatic;
5453
import com.oracle.truffle.api.dsl.Specialization;
@@ -111,8 +110,8 @@ static boolean doPythonObject(PythonAbstractObject object,
111110
return lookupIndex.execute(type) != PNone.NO_VALUE || lookupInt.execute(type) != PNone.NO_VALUE || lookupFloat.execute(type) != PNone.NO_VALUE || checkComplex.execute(object);
112111
}
113112

114-
@Fallback
115-
static boolean doObject(Object object,
113+
@Specialization(replaces = "doPythonObject", guards = {"!isDouble(object)", "!isInteger(object)", "!isBoolean(object)", "!isNone(object)", "!isString(object)"})
114+
static boolean doObject(VirtualFrame frame, Object object,
116115
@CachedLibrary(limit = "3") InteropLibrary interopLibrary,
117116
@Cached GetClassNode getClassNode,
118117
@Cached(parameters = "Index") LookupCallableSlotInMRONode lookupIndex,

0 commit comments

Comments
 (0)