Skip to content

Commit d93c5a3

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

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
@@ -145,7 +145,7 @@ PyAPI_FUNC(void) _Py_Executors_InvalidateCold(PyInterpreterState *interp);
145145

146146
int _Py_uop_analyze_and_optimize(struct _PyInterpreterFrame *frame,
147147
_PyUOpInstruction *trace, int trace_len, int curr_stackentries,
148-
_PyBloomFilter *dependencies, PyObject *refs);
148+
_PyBloomFilter *dependencies, PyObject *new_refs);
149149

150150
extern PyTypeObject _PyCounterExecutor_Type;
151151
extern PyTypeObject _PyCounterOptimizer_Type;

‎Python/optimizer.c‎

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

1136-
static PyObject *
1137-
safe_constant_key(PyObject *o)
1138-
{
1139-
PyObject *k = _PyCode_ConstantKey(o);
1140-
// A key of tuple[int, object] is used for "other" constants, which may
1141-
// include arbitrary objects. We don't want to try to hash them or check
1142-
// their equality, so just make the key a tuple[int] (their address):
1143-
if (k && PyTuple_CheckExact(k) && PyLong_CheckExact(PyTuple_GET_ITEM(k, 0))) {
1144-
Py_SETREF(k, PyTuple_Pack(1, PyTuple_GET_ITEM(k, 0)));
1145-
}
1146-
return k;
1147-
}
1148-
1149-
static bool
1150-
safe_contains(PyObject *refs, PyObject *o)
1151-
{
1152-
assert(PyList_CheckExact(refs));
1153-
for (int i = 0; i < PyList_GET_SIZE(refs); i++) {
1154-
if (Py_Is(o, PyList_GET_ITEM(refs, i))) {
1155-
return true;
1156-
}
1157-
}
1158-
return false;
1159-
}
1160-
1161-
static int
1162-
merge_const(PyObject *refs, PyObject **o_p)
1163-
{
1164-
assert(PyDict_CheckExact(refs));
1165-
assert(!_Py_IsImmortal(*o_p));
1166-
PyObject *o = *o_p;
1167-
PyObject *k = safe_constant_key(o);
1168-
if (k == NULL) {
1169-
return -1;
1170-
}
1171-
int res = PyDict_SetDefaultRef(refs, k, o, o_p);
1172-
Py_DECREF(k);
1173-
Py_DECREF(o);
1174-
return res;
1175-
}
1176-
11771136
/* Makes an executor from a buffer of uops.
11781137
* Account for the buffer having gaps and NOPs by computing a "used"
11791138
* bit vector and only copying the used uops. Here "used" means reachable
@@ -1291,11 +1250,13 @@ uop_optimize(
12911250
}
12921251
assert(length < UOP_MAX_TRACE_LENGTH);
12931252
OPT_STAT_INC(traces_created);
1294-
char *env_var = Py_GETENV("PYTHON_UOPS_OPTIMIZE");
1253+
// These are any references that were created during optimization, and need
1254+
// to be kept alive until we build the executor's refs tuple:
12951255
PyObject *new_refs = PyList_New(0);
12961256
if (new_refs == NULL) {
12971257
return -1;
12981258
}
1259+
char *env_var = Py_GETENV("PYTHON_UOPS_OPTIMIZE");
12991260
if (env_var == NULL || *env_var == '\0' || *env_var > '0') {
13001261
length = _Py_uop_analyze_and_optimize(frame, buffer,
13011262
length,
@@ -1323,45 +1284,39 @@ uop_optimize(
13231284
assert(_PyOpcode_uop_name[buffer[pc].opcode]);
13241285
assert(strncmp(_PyOpcode_uop_name[buffer[pc].opcode], _PyOpcode_uop_name[opcode], strlen(_PyOpcode_uop_name[opcode])) == 0);
13251286
}
1326-
PyObject *used_refs = PyDict_New();
1327-
if (used_refs == NULL) {
1287+
// We *might* want to de-duplicate these. In addition to making sure we do
1288+
// so in a way that preserves "equal" constants with different types (see
1289+
// _PyCode_ConstantKey), we *also* need to be careful to compare unknown
1290+
// objects by identity, since we don't want to invoke arbitary code in a
1291+
// __hash__/__eq__ implementation. It might be more trouble than it's worth:
1292+
int refs_needed = 0;
1293+
for (int i = 0; i < length; i++) {
1294+
if (buffer[i].opcode == _LOAD_CONST_INLINE) {
1295+
refs_needed++;
1296+
}
1297+
}
1298+
PyObject *refs = PyTuple_New(refs_needed);
1299+
if (refs == NULL) {
13281300
Py_DECREF(new_refs);
13291301
return -1;
13301302
}
1303+
int j = 0;
13311304
for (int i = 0; i < length; i++) {
13321305
if (buffer[i].opcode == _LOAD_CONST_INLINE) {
1333-
PyObject **o_p = (PyObject **)&buffer[i].operand;
1334-
if (!safe_contains(new_refs, *o_p)) {
1335-
continue;
1336-
}
1337-
Py_INCREF(*o_p);
1338-
int err = merge_const(used_refs, o_p);
1339-
Py_DECREF(*o_p);
1340-
if (err < 0) {
1341-
Py_DECREF(used_refs);
1342-
Py_DECREF(new_refs);
1343-
return -1;
1344-
}
1306+
PyTuple_SET_ITEM(refs, j++, Py_NewRef(buffer[i].operand));
13451307
}
13461308
}
13471309
Py_DECREF(new_refs);
1348-
Py_SETREF(used_refs, PyDict_Values(used_refs));
1349-
if (used_refs == NULL) {
1350-
return -1;
1351-
}
1352-
Py_SETREF(used_refs, PyList_AsTuple(used_refs));
1353-
if (used_refs == NULL) {
1354-
return -1;
1355-
}
1310+
assert(j == refs_needed);
13561311
OPT_HIST(effective_trace_length(buffer, length), optimized_trace_length_hist);
13571312
length = prepare_for_execution(buffer, length);
13581313
assert(length <= UOP_MAX_TRACE_LENGTH);
13591314
_PyExecutorObject *executor = make_executor_from_uops(buffer, length, &dependencies);
13601315
if (executor == NULL) {
1361-
Py_DECREF(used_refs);
1316+
Py_DECREF(refs);
13621317
return -1;
13631318
}
1364-
executor->refs = used_refs;
1319+
executor->refs = refs;
13651320
assert(length <= UOP_MAX_TRACE_LENGTH);
13661321
*exec_ptr = executor;
13671322
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)