Skip to content

gh-116996: Add pystats about _Py_uop_analyse_and_optimize #116997

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Mar 21, 2024
Merged

gh-116996: Add pystats about _Py_uop_analyse_and_optimize #116997

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8658072
gh-116996: Add pystats about _Py_uop_analyse_and_optimize
mdboom Mar 19, 2024
6605b43
Fix build and address feedback
mdboom Mar 20, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion Include/cpython/pystats.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@
// Define _PY_INTERPRETER macro to increment interpreter_increfs and
// interpreter_decrefs. Otherwise, increment increfs and decrefs.

#include "pycore_uop_ids.h"

#ifndef Py_CPYTHON_PYSTATS_H
# error "this header file must not be included directly"
#endif
Expand Down Expand Up @@ -116,14 +118,17 @@ typedef struct _optimization_stats {
uint64_t recursive_call;
uint64_t low_confidence;
uint64_t executors_invalidated;
UOpStats opcode[512];
UOpStats opcode[MAX_UOP_ID];
uint64_t unsupported_opcode[256];
uint64_t trace_length_hist[_Py_UOP_HIST_SIZE];
uint64_t trace_run_length_hist[_Py_UOP_HIST_SIZE];
uint64_t optimized_trace_length_hist[_Py_UOP_HIST_SIZE];
uint64_t optimizer_attempts;
uint64_t optimizer_successes;
uint64_t optimizer_failure_reason_no_memory;
uint64_t remove_globals_builtins_changed;
uint64_t remove_globals_incorrect_keys;
uint64_t error_in_opcode[MAX_UOP_ID];
} OptimizationStats;

typedef struct _rare_event_stats {
Expand Down
2 changes: 2 additions & 0 deletions Include/internal/pycore_code.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -308,6 +308,7 @@ extern int _PyStaticCode_Init(PyCodeObject *co);
#define OPT_STAT_INC(name) do { if (_Py_stats) _Py_stats->optimization_stats.name++; } while (0)
#define UOP_STAT_INC(opname, name) do { if (_Py_stats) { assert(opname < 512); _Py_stats->optimization_stats.opcode[opname].name++; } } while (0)
#define OPT_UNSUPPORTED_OPCODE(opname) do { if (_Py_stats) _Py_stats->optimization_stats.unsupported_opcode[opname]++; } while (0)
#define OPT_ERROR_IN_OPCODE(opname) do { if (_Py_stats) _Py_stats->optimization_stats.error_in_opcode[opname]++; } while (0)
#define OPT_HIST(length, name) \
do { \
if (_Py_stats) { \
Expand All @@ -334,6 +335,7 @@ PyAPI_FUNC(PyObject*) _Py_GetSpecializationStats(void);
#define OPT_STAT_INC(name) ((void)0)
#define UOP_STAT_INC(opname, name) ((void)0)
#define OPT_UNSUPPORTED_OPCODE(opname) ((void)0)
#define OPT_ERROR_IN_OPCODE(opname) ((void)0)
#define OPT_HIST(length, name) ((void)0)
#define RARE_EVENT_STAT_INC(name) ((void)0)
#endif // !Py_STATS
Expand Down
11 changes: 9 additions & 2 deletions Python/optimizer_analysis.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,7 @@ remove_globals(_PyInterpreterFrame *frame, _PyUOpInstruction *buffer,
PyInterpreterState *interp = _PyInterpreterState_GET();
PyObject *builtins = frame->f_builtins;
if (builtins != interp->builtins) {
OPT_STAT_INC(remove_globals_builtins_changed);
return 1;
}
PyObject *globals = frame->f_globals;
Expand Down Expand Up @@ -170,6 +171,7 @@ remove_globals(_PyInterpreterFrame *frame, _PyUOpInstruction *buffer,
switch(opcode) {
case _GUARD_BUILTINS_VERSION:
if (incorrect_keys(inst, builtins)) {
OPT_STAT_INC(remove_globals_incorrect_keys);
return 0;
}
if (interp->rare_events.builtin_dict >= _Py_MAX_ALLOWED_BUILTINS_MODIFICATIONS) {
Expand All @@ -190,6 +192,7 @@ remove_globals(_PyInterpreterFrame *frame, _PyUOpInstruction *buffer,
break;
case _GUARD_GLOBALS_VERSION:
if (incorrect_keys(inst, globals)) {
OPT_STAT_INC(remove_globals_incorrect_keys);
return 0;
}
uint64_t watched_mutations = get_mutations(globals);
Expand Down Expand Up @@ -238,6 +241,7 @@ remove_globals(_PyInterpreterFrame *frame, _PyUOpInstruction *buffer,
globals = func->func_globals;
builtins = func->func_builtins;
if (builtins != interp->builtins) {
OPT_STAT_INC(remove_globals_builtins_changed);
return 1;
}
break;
Expand Down Expand Up @@ -358,6 +362,7 @@ optimize_uops(

_Py_UOpsContext context;
_Py_UOpsContext *ctx = &context;
uint32_t opcode = UINT16_MAX;

if (_Py_uop_abstractcontext_init(ctx) < 0) {
goto out_of_space;
Expand All @@ -374,8 +379,7 @@ optimize_uops(
this_instr++) {

int oparg = this_instr->oparg;
uint32_t opcode = this_instr->opcode;

opcode = this_instr->opcode;
_Py_UopsSymbol **stack_pointer = ctx->frame->stack_pointer;

#ifdef Py_DEBUG
Expand Down Expand Up @@ -412,6 +416,9 @@ optimize_uops(
error:
DPRINTF(3, "\n");
DPRINTF(1, "Encountered error in abstract interpreter\n");
if (opcode <= MAX_UOP_ID) {
Copy link
Member

@Fidget-Spinner Fidget-Spinner Mar 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is somewhat concerning, on one hand it's bounded by 512, then here it's bounded by MAX_UOP_ID. MAX_UOP_ID might grow to above 512, so I think bounding by either one and sticking to it is better.

Copy link
Contributor Author

@mdboom mdboom Mar 20, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense.

OPT_ERROR_IN_OPCODE(opcode);
Copy link
Member

@Fidget-Spinner Fidget-Spinner Mar 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You forgot to define this I think.

Copy link
Contributor Author

@mdboom mdboom Mar 20, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, indeed I forgot to commit it.

}
_Py_uop_abstractcontext_fini(ctx);
return 0;

Expand Down
13 changes: 13 additions & 0 deletions Python/specialize.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -247,6 +247,8 @@ print_optimization_stats(FILE *out, OptimizationStats *stats)
fprintf(out, "Optimization optimizer successes: %" PRIu64 "\n", stats->optimizer_successes);
fprintf(out, "Optimization optimizer failure no memory: %" PRIu64 "\n",
stats->optimizer_failure_reason_no_memory);
fprintf(out, "Optimizer remove globals builtins changed: %" PRIu64 "\n", stats->remove_globals_builtins_changed);
fprintf(out, "Optimizer remove globals incorrect keys: %" PRIu64 "\n", stats->remove_globals_incorrect_keys);

const char* const* names;
for (int i = 0; i <= MAX_UOP_ID; i++) {
Expand All @@ -268,6 +270,17 @@ print_optimization_stats(FILE *out, OptimizationStats *stats)
);
}
}

for (int i = 0; i < MAX_UOP_ID; i++) {
if (stats->error_in_opcode[i]) {
fprintf(
out,
"error_in_opcode[%s].count : %" PRIu64 "\n",
_PyUOpName(i),
stats->error_in_opcode[i]
);
}
}
}

static void
Expand Down
26 changes: 26 additions & 0 deletions Tools/scripts/summarize_stats.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -513,6 +513,8 @@ def get_optimizer_stats(self) -> dict[str, tuple[int, int | None]]:
attempts = self._data["Optimization optimizer attempts"]
successes = self._data["Optimization optimizer successes"]
no_memory = self._data["Optimization optimizer failure no memory"]
builtins_changed = self._data["Optimizer remove globals builtins changed"]
incorrect_keys = self._data["Optimizer remove globals incorrect keys"]

return {
Doc(
Expand All @@ -527,6 +529,14 @@ def get_optimizer_stats(self) -> dict[str, tuple[int, int | None]]:
"Optimizer no memory",
"The number of optimizations that failed due to no memory.",
): (no_memory, attempts),
Doc(
"Remove globals builtins changed",
"The builtins changed during optimization",
): (builtins_changed, attempts),
Doc(
"Remove globals incorrect keys",
"The keys in the globals dictionary aren't what was expected",
): (incorrect_keys, attempts),
}

def get_histogram(self, prefix: str) -> list[tuple[int, int]]:
Expand Down Expand Up @@ -1177,6 +1187,17 @@ def calc_unsupported_opcodes_table(stats: Stats) -> Rows:
reverse=True,
)

def calc_error_in_opcodes_table(stats: Stats) -> Rows:
error_in_opcodes = stats.get_opcode_stats("error_in_opcode")
return sorted(
[
(opcode, Count(count))
for opcode, count in error_in_opcodes.get_opcode_counts().items()
],
key=itemgetter(1),
reverse=True,
)

def iter_optimization_tables(base_stats: Stats, head_stats: Stats | None = None):
if not base_stats.get_optimization_stats() or (
head_stats is not None and not head_stats.get_optimization_stats()
Expand Down Expand Up @@ -1223,6 +1244,11 @@ def iter_optimization_tables(base_stats: Stats, head_stats: Stats | None = None)
)
],
)
yield Section(
"Optimizer errored out with opcode",
"Optimization stopped after encountering this opcode",
[Table(("Opcode", "Count:"), calc_error_in_opcodes_table, JoinMode.CHANGE)],
)

return Section(
"Optimization (Tier 2) stats",
Expand Down