Skip to content

Commit eb54546

Browse files
brandtbucherbrandtbucher
committed
Don't bother deduplicating (and factor out some repeated code)
1 parent 9c47526 commit eb54546

File tree

5 files changed

+108
-233
lines changed

5 files changed

+108
-233
lines changed

‎Include/internal/pycore_optimizer.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,7 @@ PyAPI_FUNC(void) _Py_Executors_InvalidateAll(PyInterpreterState *interp, int is_
137137

138138
int _Py_uop_analyze_and_optimize(struct _PyInterpreterFrame *frame,
139139
_PyUOpInstruction *trace, int trace_len, int curr_stackentries,
140-
_PyBloomFilter *dependencies, PyObject *refs);
140+
_PyBloomFilter *dependencies, PyObject *new_refs);
141141

142142
extern PyTypeObject _PyCounterExecutor_Type;
143143
extern PyTypeObject _PyCounterOptimizer_Type;

‎Python/optimizer.c‎

Lines changed: 21 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -1132,47 +1132,6 @@ sanity_check(_PyExecutorObject *executor)
11321132
#undef CHECK
11331133
#endif
11341134

1135-
static PyObject *
1136-
safe_constant_key(PyObject *o)
1137-
{
1138-
PyObject *k = _PyCode_ConstantKey(o);
1139-
// A key of tuple[int, object] is used for "other" constants, which may
1140-
// include arbitrary objects. We don't want to try to hash them or check
1141-
// their equality, so just make the key a tuple[int] (their address):
1142-
if (k && PyTuple_CheckExact(k) && PyLong_CheckExact(PyTuple_GET_ITEM(k, 0))) {
1143-
Py_SETREF(k, PyTuple_Pack(1, PyTuple_GET_ITEM(k, 0)));
1144-
}
1145-
return k;
1146-
}
1147-
1148-
static bool
1149-
safe_contains(PyObject *refs, PyObject *o)
1150-
{
1151-
assert(PyList_CheckExact(refs));
1152-
for (int i = 0; i < PyList_GET_SIZE(refs); i++) {
1153-
if (Py_Is(o, PyList_GET_ITEM(refs, i))) {
1154-
return true;
1155-
}
1156-
}
1157-
return false;
1158-
}
1159-
1160-
static int
1161-
merge_const(PyObject *refs, PyObject **o_p)
1162-
{
1163-
assert(PyDict_CheckExact(refs));
1164-
assert(!_Py_IsImmortal(*o_p));
1165-
PyObject *o = *o_p;
1166-
PyObject *k = safe_constant_key(o);
1167-
if (k == NULL) {
1168-
return -1;
1169-
}
1170-
int res = PyDict_SetDefaultRef(refs, k, o, o_p);
1171-
Py_DECREF(k);
1172-
Py_DECREF(o);
1173-
return res;
1174-
}
1175-
11761135
/* Makes an executor from a buffer of uops.
11771136
* Account for the buffer having gaps and NOPs by computing a "used"
11781137
* bit vector and only copying the used uops. Here "used" means reachable
@@ -1287,11 +1246,13 @@ uop_optimize(
12871246
}
12881247
assert(length < UOP_MAX_TRACE_LENGTH);
12891248
OPT_STAT_INC(traces_created);
1290-
char *env_var = Py_GETENV("PYTHON_UOPS_OPTIMIZE");
1249+
// These are any references that were created during optimization, and need
1250+
// to be kept alive until we build the executor's refs tuple:
12911251
PyObject *new_refs = PyList_New(0);
12921252
if (new_refs == NULL) {
12931253
return -1;
12941254
}
1255+
char *env_var = Py_GETENV("PYTHON_UOPS_OPTIMIZE");
12951256
if (env_var == NULL || *env_var == '\0' || *env_var > '0') {
12961257
length = _Py_uop_analyze_and_optimize(frame, buffer,
12971258
length,
@@ -1319,45 +1280,39 @@ uop_optimize(
13191280
assert(_PyOpcode_uop_name[buffer[pc].opcode]);
13201281
assert(strncmp(_PyOpcode_uop_name[buffer[pc].opcode], _PyOpcode_uop_name[opcode], strlen(_PyOpcode_uop_name[opcode])) == 0);
13211282
}
1322-
PyObject *used_refs = PyDict_New();
1323-
if (used_refs == NULL) {
1283+
// We *might* want to de-duplicate these. In addition to making sure we do
1284+
// so in a way that preserves "equal" constants with different types (see
1285+
// _PyCode_ConstantKey), we *also* need to be careful to compare unknown
1286+
// objects by identity, since we don't want to invoke arbitary code in a
1287+
// __hash__/__eq__ implementation. It might be more trouble than it's worth:
1288+
int refs_needed = 0;
1289+
for (int i = 0; i < length; i++) {
1290+
if (buffer[i].opcode == _LOAD_CONST_INLINE) {
1291+
refs_needed++;
1292+
}
1293+
}
1294+
PyObject *refs = PyTuple_New(refs_needed);
1295+
if (refs == NULL) {
13241296
Py_DECREF(new_refs);
13251297
return -1;
13261298
}
1299+
int j = 0;
13271300
for (int i = 0; i < length; i++) {
13281301
if (buffer[i].opcode == _LOAD_CONST_INLINE) {
1329-
PyObject **o_p = (PyObject **)&buffer[i].operand;
1330-
if (!safe_contains(new_refs, *o_p)) {
1331-
continue;
1332-
}
1333-
Py_INCREF(*o_p);
1334-
int err = merge_const(used_refs, o_p);
1335-
Py_DECREF(*o_p);
1336-
if (err < 0) {
1337-
Py_DECREF(used_refs);
1338-
Py_DECREF(new_refs);
1339-
return -1;
1340-
}
1302+
PyTuple_SET_ITEM(refs, j++, Py_NewRef(buffer[i].operand));
13411303
}
13421304
}
13431305
Py_DECREF(new_refs);
1344-
Py_SETREF(used_refs, PyDict_Values(used_refs));
1345-
if (used_refs == NULL) {
1346-
return -1;
1347-
}
1348-
Py_SETREF(used_refs, PyList_AsTuple(used_refs));
1349-
if (used_refs == NULL) {
1350-
return -1;
1351-
}
1306+
assert(j == refs_needed);
13521307
OPT_HIST(effective_trace_length(buffer, length), optimized_trace_length_hist);
13531308
length = prepare_for_execution(buffer, length);
13541309
assert(length <= UOP_MAX_TRACE_LENGTH);
13551310
_PyExecutorObject *executor = make_executor_from_uops(buffer, length, &dependencies);
13561311
if (executor == NULL) {
1357-
Py_DECREF(used_refs);
1312+
Py_DECREF(refs);
13581313
return -1;
13591314
}
1360-
executor->refs = used_refs;
1315+
executor->refs = refs;
13611316
assert(length <= UOP_MAX_TRACE_LENGTH);
13621317
*exec_ptr = executor;
13631318
return 1;

‎Python/optimizer_analysis.c‎

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -300,10 +300,20 @@ remove_globals(_PyInterpreterFrame *frame, _PyUOpInstruction *buffer,
300300

301301
#define GETLOCAL(idx) ((ctx->frame->locals[idx]))
302302

303-
#define REPLACE_OP(INST, OP, ARG, OPERAND) \
304-
(INST)->opcode = OP; \
305-
(INST)->oparg = ARG; \
306-
(INST)->operand = OPERAND;
303+
#define REPLACE_OP(INST, OP, ARG, OPERAND) \
304+
do { \
305+
(INST)->opcode = (OP); \
306+
(INST)->oparg = (ARG); \
307+
(INST)->operand = (OPERAND); \
308+
} while (0)
309+
310+
#define REPLACE_OP_WITH_LOAD_CONST(INST, CONST) \
311+
do { \
312+
PyObject *o = (CONST); \
313+
int opcode = _Py_IsImmortal(o) ? _LOAD_CONST_INLINE_BORROW \
314+
: _LOAD_CONST_INLINE; \
315+
REPLACE_OP((INST), opcode, 0, (uintptr_t)o); \
316+
} while (0)
307317

308318
/* Shortened forms for convenience, used in optimizer_bytecodes.c */
309319
#define sym_is_not_null _Py_uop_sym_is_not_null

‎Python/optimizer_bytecodes.c‎

Lines changed: 36 additions & 81 deletions
Original file line numberDiff line numberDiff line change
@@ -83,9 +83,7 @@ dummy_func(void) {
8383
op(_LOAD_FAST, (-- value)) {
8484
value = GETLOCAL(oparg);
8585
if (sym_is_const(value)) {
86-
PyObject *val = sym_get_const(value);
87-
int opcode = _Py_IsImmortal(val) ? _LOAD_CONST_INLINE_BORROW : _LOAD_CONST_INLINE;
88-
REPLACE_OP(this_instr, opcode, 0, (uintptr_t)val);
86+
REPLACE_OP_WITH_LOAD_CONST(this_instr, sym_get_const(value));
8987
}
9088
}
9189

@@ -199,20 +197,15 @@ dummy_func(void) {
199197
assert(PyLong_CheckExact(sym_get_const(right)));
200198
PyObject *temp = _PyLong_Add((PyLongObject *)sym_get_const(left),
201199
(PyLongObject *)sym_get_const(right));
202-
if (temp == NULL) {
200+
if (temp == NULL || _PyList_AppendTakeRef((PyListObject *)new_refs, temp)) {
203201
goto error;
204202
}
205203
assert(this_instr[-2].opcode == _NOP);
206204
assert(this_instr[-1].opcode == _NOP);
207-
REPLACE_OP(&this_instr[-2], _POP_TOP, 0, 0);
208-
REPLACE_OP(&this_instr[-1], _POP_TOP, 0, 0);
209-
if (PyList_Append(new_refs, temp)) {
210-
goto error;
211-
}
212-
int opcode = _Py_IsImmortal(temp) ? _LOAD_CONST_INLINE_BORROW : _LOAD_CONST_INLINE;
213-
REPLACE_OP(this_instr, opcode, 0, (uintptr_t)temp);
205+
REPLACE_OP(this_instr - 2, _POP_TOP, 0, 0);
206+
REPLACE_OP(this_instr - 1, _POP_TOP, 0, 0);
207+
REPLACE_OP_WITH_LOAD_CONST(this_instr, temp);
214208
res = sym_new_const(ctx, temp);
215-
Py_DECREF(temp);
216209
}
217210
else {
218211
res = sym_new_type(ctx, &PyLong_Type);
@@ -227,20 +220,15 @@ dummy_func(void) {
227220
assert(PyLong_CheckExact(sym_get_const(right)));
228221
PyObject *temp = _PyLong_Subtract((PyLongObject *)sym_get_const(left),
229222
(PyLongObject *)sym_get_const(right));
230-
if (temp == NULL) {
223+
if (temp == NULL || _PyList_AppendTakeRef((PyListObject *)new_refs, temp)) {
231224
goto error;
232225
}
233226
assert(this_instr[-2].opcode == _NOP);
234227
assert(this_instr[-1].opcode == _NOP);
235-
REPLACE_OP(&this_instr[-2], _POP_TOP, 0, 0);
236-
REPLACE_OP(&this_instr[-1], _POP_TOP, 0, 0);
237-
if (PyList_Append(new_refs, temp)) {
238-
goto error;
239-
}
240-
int opcode = _Py_IsImmortal(temp) ? _LOAD_CONST_INLINE_BORROW : _LOAD_CONST_INLINE;
241-
REPLACE_OP(this_instr, opcode, 0, (uintptr_t)temp);
228+
REPLACE_OP(this_instr - 2, _POP_TOP, 0, 0);
229+
REPLACE_OP(this_instr - 1, _POP_TOP, 0, 0);
230+
REPLACE_OP_WITH_LOAD_CONST(this_instr, temp);
242231
res = sym_new_const(ctx, temp);
243-
Py_DECREF(temp);
244232
}
245233
else {
246234
res = sym_new_type(ctx, &PyLong_Type);
@@ -255,20 +243,15 @@ dummy_func(void) {
255243
assert(PyLong_CheckExact(sym_get_const(right)));
256244
PyObject *temp = _PyLong_Multiply((PyLongObject *)sym_get_const(left),
257245
(PyLongObject *)sym_get_const(right));
258-
if (temp == NULL) {
246+
if (temp == NULL || _PyList_AppendTakeRef((PyListObject *)new_refs, temp)) {
259247
goto error;
260248
}
261249
assert(this_instr[-2].opcode == _NOP);
262250
assert(this_instr[-1].opcode == _NOP);
263-
REPLACE_OP(&this_instr[-2], _POP_TOP, 0, 0);
264-
REPLACE_OP(&this_instr[-1], _POP_TOP, 0, 0);
265-
if (PyList_Append(new_refs, temp)) {
266-
goto error;
267-
}
268-
int opcode = _Py_IsImmortal(temp) ? _LOAD_CONST_INLINE_BORROW : _LOAD_CONST_INLINE;
269-
REPLACE_OP(this_instr, opcode, 0, (uintptr_t)temp);
251+
REPLACE_OP(this_instr - 2, _POP_TOP, 0, 0);
252+
REPLACE_OP(this_instr - 1, _POP_TOP, 0, 0);
253+
REPLACE_OP_WITH_LOAD_CONST(this_instr, temp);
270254
res = sym_new_const(ctx, temp);
271-
Py_DECREF(temp);
272255
}
273256
else {
274257
res = sym_new_type(ctx, &PyLong_Type);
@@ -284,20 +267,15 @@ dummy_func(void) {
284267
PyObject *temp = PyFloat_FromDouble(
285268
PyFloat_AS_DOUBLE(sym_get_const(left)) +
286269
PyFloat_AS_DOUBLE(sym_get_const(right)));
287-
if (temp == NULL) {
270+
if (temp == NULL || _PyList_AppendTakeRef((PyListObject *)new_refs, temp)) {
288271
goto error;
289272
}
290273
assert(this_instr[-2].opcode == _NOP);
291274
assert(this_instr[-1].opcode == _NOP);
292-
REPLACE_OP(&this_instr[-2], _POP_TOP, 0, 0);
293-
REPLACE_OP(&this_instr[-1], _POP_TOP, 0, 0);
294-
if (PyList_Append(new_refs, temp)) {
295-
goto error;
296-
}
297-
int opcode = _Py_IsImmortal(temp) ? _LOAD_CONST_INLINE_BORROW : _LOAD_CONST_INLINE;
298-
REPLACE_OP(this_instr, opcode, 0, (uintptr_t)temp);
275+
REPLACE_OP(this_instr - 2, _POP_TOP, 0, 0);
276+
REPLACE_OP(this_instr - 1, _POP_TOP, 0, 0);
277+
REPLACE_OP_WITH_LOAD_CONST(this_instr, temp);
299278
res = sym_new_const(ctx, temp);
300-
Py_DECREF(temp);
301279
}
302280
else {
303281
res = sym_new_type(ctx, &PyFloat_Type);
@@ -313,20 +291,15 @@ dummy_func(void) {
313291
PyObject *temp = PyFloat_FromDouble(
314292
PyFloat_AS_DOUBLE(sym_get_const(left)) -
315293
PyFloat_AS_DOUBLE(sym_get_const(right)));
316-
if (temp == NULL) {
294+
if (temp == NULL || _PyList_AppendTakeRef((PyListObject *)new_refs, temp)) {
317295
goto error;
318296
}
319297
assert(this_instr[-2].opcode == _NOP);
320298
assert(this_instr[-1].opcode == _NOP);
321-
REPLACE_OP(&this_instr[-2], _POP_TOP, 0, 0);
322-
REPLACE_OP(&this_instr[-1], _POP_TOP, 0, 0);
323-
if (PyList_Append(new_refs, temp)) {
324-
goto error;
325-
}
326-
int opcode = _Py_IsImmortal(temp) ? _LOAD_CONST_INLINE_BORROW : _LOAD_CONST_INLINE;
327-
REPLACE_OP(this_instr, opcode, 0, (uintptr_t)temp);
299+
REPLACE_OP(this_instr - 2, _POP_TOP, 0, 0);
300+
REPLACE_OP(this_instr - 1, _POP_TOP, 0, 0);
301+
REPLACE_OP_WITH_LOAD_CONST(this_instr, temp);
328302
res = sym_new_const(ctx, temp);
329-
Py_DECREF(temp);
330303
}
331304
else {
332305
res = sym_new_type(ctx, &PyFloat_Type);
@@ -342,20 +315,15 @@ dummy_func(void) {
342315
PyObject *temp = PyFloat_FromDouble(
343316
PyFloat_AS_DOUBLE(sym_get_const(left)) *
344317
PyFloat_AS_DOUBLE(sym_get_const(right)));
345-
if (temp == NULL) {
318+
if (temp == NULL || _PyList_AppendTakeRef((PyListObject *)new_refs, temp)) {
346319
goto error;
347320
}
348321
assert(this_instr[-2].opcode == _NOP);
349322
assert(this_instr[-1].opcode == _NOP);
350-
REPLACE_OP(&this_instr[-2], _POP_TOP, 0, 0);
351-
REPLACE_OP(&this_instr[-1], _POP_TOP, 0, 0);
352-
if (PyList_Append(new_refs, temp)) {
353-
goto error;
354-
}
355-
int opcode = _Py_IsImmortal(temp) ? _LOAD_CONST_INLINE_BORROW : _LOAD_CONST_INLINE;
356-
REPLACE_OP(this_instr, opcode, 0, (uintptr_t)temp);
323+
REPLACE_OP(this_instr - 2, _POP_TOP, 0, 0);
324+
REPLACE_OP(this_instr - 1, _POP_TOP, 0, 0);
325+
REPLACE_OP_WITH_LOAD_CONST(this_instr, temp);
357326
res = sym_new_const(ctx, temp);
358-
Py_DECREF(temp);
359327
}
360328
else {
361329
res = sym_new_type(ctx, &PyFloat_Type);
@@ -366,20 +334,15 @@ dummy_func(void) {
366334
if (sym_is_const(left) && sym_is_const(right) &&
367335
sym_matches_type(left, &PyUnicode_Type) && sym_matches_type(right, &PyUnicode_Type)) {
368336
PyObject *temp = PyUnicode_Concat(sym_get_const(left), sym_get_const(right));
369-
if (temp == NULL) {
337+
if (temp == NULL || _PyList_AppendTakeRef((PyListObject *)new_refs, temp)) {
370338
goto error;
371339
}
372340
assert(this_instr[-2].opcode == _NOP);
373341
assert(this_instr[-1].opcode == _NOP);
374-
REPLACE_OP(&this_instr[-2], _POP_TOP, 0, 0);
375-
REPLACE_OP(&this_instr[-1], _POP_TOP, 0, 0);
376-
if (PyList_Append(new_refs, temp)) {
377-
goto error;
378-
}
379-
int opcode = _Py_IsImmortal(temp) ? _LOAD_CONST_INLINE_BORROW : _LOAD_CONST_INLINE;
380-
REPLACE_OP(this_instr, opcode, 0, (uintptr_t)temp);
342+
REPLACE_OP(this_instr - 2, _POP_TOP, 0, 0);
343+
REPLACE_OP(this_instr - 1, _POP_TOP, 0, 0);
344+
REPLACE_OP_WITH_LOAD_CONST(this_instr, temp);
381345
res = sym_new_const(ctx, temp);
382-
Py_DECREF(temp);
383346
}
384347
else {
385348
res = sym_new_type(ctx, &PyUnicode_Type);
@@ -391,22 +354,17 @@ dummy_func(void) {
391354
if (sym_is_const(left) && sym_is_const(right) &&
392355
sym_matches_type(left, &PyUnicode_Type) && sym_matches_type(right, &PyUnicode_Type)) {
393356
PyObject *temp = PyUnicode_Concat(sym_get_const(left), sym_get_const(right));
394-
if (temp == NULL) {
357+
if (temp == NULL || _PyList_AppendTakeRef((PyListObject *)new_refs, temp)) {
395358
goto error;
396359
}
397360
assert(this_instr[-3].opcode == _NOP);
398361
assert(this_instr[-2].opcode == _NOP);
399362
assert(this_instr[-1].opcode == _NOP);
400-
REPLACE_OP(&this_instr[-3], _POP_TOP, 0, 0);
401-
REPLACE_OP(&this_instr[-2], _POP_TOP, 0, 0);
402-
if (PyList_Append(new_refs, temp)) {
403-
goto error;
404-
}
405-
int opcode = _Py_IsImmortal(temp) ? _LOAD_CONST_INLINE_BORROW : _LOAD_CONST_INLINE;
406-
REPLACE_OP(&this_instr[-1], opcode, 0, (uintptr_t)temp);
407-
res = sym_new_const(ctx, temp);
408-
Py_DECREF(temp);
363+
REPLACE_OP(this_instr - 3, _POP_TOP, 0, 0);
364+
REPLACE_OP(this_instr - 2, _POP_TOP, 0, 0);
365+
REPLACE_OP_WITH_LOAD_CONST(this_instr - 1, temp);
409366
REPLACE_OP(this_instr, _STORE_FAST, this_instr->operand, 0);
367+
res = sym_new_const(ctx, temp);
410368
}
411369
else {
412370
res = sym_new_type(ctx, &PyUnicode_Type);
@@ -505,8 +463,7 @@ dummy_func(void) {
505463

506464
op(_LOAD_CONST, (-- value)) {
507465
PyObject *val = PyTuple_GET_ITEM(co->co_consts, this_instr->oparg);
508-
int opcode = _Py_IsImmortal(val) ? _LOAD_CONST_INLINE_BORROW : _LOAD_CONST_INLINE;
509-
REPLACE_OP(this_instr, opcode, 0, (uintptr_t)val);
466+
REPLACE_OP_WITH_LOAD_CONST(this_instr, val);
510467
value = sym_new_const(ctx, val);
511468
}
512469

@@ -530,9 +487,7 @@ dummy_func(void) {
530487

531488
op(_COPY, (bottom, unused[oparg-1] -- bottom, unused[oparg-1], top)) {
532489
if (sym_is_const(bottom)) {
533-
PyObject *value = sym_get_const(bottom);
534-
int opcode = _Py_IsImmortal(value) ? _LOAD_CONST_INLINE_BORROW : _LOAD_CONST_INLINE;
535-
REPLACE_OP(this_instr, opcode, 0, (uintptr_t)value);
490+
REPLACE_OP_WITH_LOAD_CONST(this_instr, sym_get_const(bottom));
536491
}
537492
assert(oparg > 0);
538493
top = bottom;

0 commit comments

Comments
 (0)