Change bucketing. Change how execution length is calculated

mdboom · mdboom · commit 69a6e7bdd5d7 · 2023-10-02T17:08:16.000-04:00
diff --git a/Include/cpython/pystats.h b/Include/cpython/pystats.h
@@ -100,10 +100,7 @@ typedef struct _uop_stats {
     uint64_t execution_count;
 } UOpStats;
 
-// Note that these should be set to support the largest possible trace length.
-// i.e. log2(MAX_LENGTH) + 1 === (_Py_UOP_HIST_SIZE + _Py_UOP_HIST_BIAS)
-#define _Py_UOP_HIST_SIZE 5
-#define _Py_UOP_HIST_BIAS 2
+#define _Py_UOP_HIST_SIZE 32
 
 typedef struct _optimization_stats {
     uint64_t attempts;
diff --git a/Include/internal/pycore_code.h b/Include/internal/pycore_code.h
@@ -285,7 +285,14 @@ extern int _PyStaticCode_Init(PyCodeObject *co);
 #define OPT_STAT_INC(name) do { if (_Py_stats) _Py_stats->optimization_stats.name++; } while (0)
 #define UOP_EXE_INC(opname) do { if (_Py_stats) _Py_stats->optimization_stats.opcode[opname].execution_count++; } while (0)
 #define OPT_UNSUPPORTED_OPCODE(opname) do { if (_Py_stats) _Py_stats->optimization_stats.unsupported_opcode[opname]++; } while (0)
-#define OPT_HIST(length, name) do { if (_Py_stats) _Py_stats->optimization_stats.name[_Py_bit_length((length - 1) >> _Py_UOP_HIST_BIAS)]++; } while (0)
+#define OPT_HIST(length, name) \
+    do { \
+        if (_Py_stats) { \
+            int bucket = _Py_bit_length(length - 1); \
+            bucket = (bucket >= _Py_UOP_HIST_SIZE) ? _Py_UOP_HIST_SIZE - 1 : bucket; \
+            _Py_stats->optimization_stats.name[bucket]++; \
+        } \
+    } while (0)
 
 // Export for '_opcode' shared extension
 PyAPI_FUNC(PyObject*) _Py_GetSpecializationStats(void);
diff --git a/Python/bytecodes.c b/Python/bytecodes.c
@@ -3913,7 +3913,7 @@ dummy_func(
             frame->prev_instr--;  // Back up to just before destination
             _PyFrame_SetStackPointer(frame, stack_pointer);
             Py_DECREF(self);
-            OPT_HIST(pc, trace_run_length_hist);
+            OPT_HIST(trace_uop_execution_counter, trace_run_length_hist);
             return frame;
         }
 
diff --git a/Python/executor.c b/Python/executor.c
@@ -79,6 +79,9 @@ _PyUopExecute(_PyExecutorObject *executor, _PyInterpreterFrame *frame, PyObject
     int opcode;
     int oparg;
     uint64_t operand;
+#ifdef Py_STATS
+    uint64_t trace_uop_execution_counter = 0;
+#endif
 
     for (;;) {
         opcode = self->trace[pc].opcode;
@@ -94,6 +97,10 @@ _PyUopExecute(_PyExecutorObject *executor, _PyInterpreterFrame *frame, PyObject
         pc++;
         OPT_STAT_INC(uops_executed);
         UOP_EXE_INC(opcode);
+#ifdef Py_STATS
+        trace_uop_execution_counter++;
+#endif
+
         switch (opcode) {
 
 #include "executor_cases.c.h"
@@ -126,7 +133,7 @@ _PyUopExecute(_PyExecutorObject *executor, _PyInterpreterFrame *frame, PyObject
     // On ERROR_IF we return NULL as the frame.
     // The caller recovers the frame from tstate->current_frame.
     DPRINTF(2, "Error: [Opcode %d, operand %" PRIu64 "]\n", opcode, operand);
-    OPT_HIST(pc, trace_run_length_hist);
+    OPT_HIST(trace_uop_execution_counter, trace_run_length_hist);
     _PyFrame_SetStackPointer(frame, stack_pointer);
     Py_DECREF(self);
     return NULL;
@@ -135,7 +142,7 @@ _PyUopExecute(_PyExecutorObject *executor, _PyInterpreterFrame *frame, PyObject
     // On DEOPT_IF we just repeat the last instruction.
     // This presumes nothing was popped from the stack (nor pushed).
     DPRINTF(2, "DEOPT: [Opcode %d, operand %" PRIu64 "]\n", opcode, operand);
-    OPT_HIST(pc, trace_run_length_hist);
+    OPT_HIST(trace_uop_execution_counter, trace_run_length_hist);
     frame->prev_instr--;  // Back up to just before destination
     _PyFrame_SetStackPointer(frame, stack_pointer);
     Py_DECREF(self);
diff --git a/Python/executor_cases.c.h b/Python/executor_cases.c.h
diff --git a/Python/specialize.c b/Python/specialize.c
@@ -215,7 +215,7 @@ static void
 print_histogram(FILE *out, const char *name, uint64_t hist[_Py_UOP_HIST_SIZE])
 {
     for (int i = 0; i < _Py_UOP_HIST_SIZE; i++) {
-        fprintf(out, "%s[%d]: %" PRIu64 "\n", name, (1 << (i + _Py_UOP_HIST_BIAS)), hist[i]);
+        fprintf(out, "%s[%" PRIu64"]: %" PRIu64 "\n", name, (uint64_t)1 << i, hist[i]);
     }
 }
 
diff --git a/Tools/scripts/summarize_stats.py b/Tools/scripts/summarize_stats.py

Original file line number	Diff line number	Diff line change
`@@ -3913,7 +3913,7 @@ dummy_func(`
`3913`	`3913`	`frame->prev_instr--; // Back up to just before destination`
`3914`	`3914`	`_PyFrame_SetStackPointer(frame, stack_pointer);`
`3915`	`3915`	`Py_DECREF(self);`
`3916`		`- OPT_HIST(pc, trace_run_length_hist);`
	`3916`	`+ OPT_HIST(trace_uop_execution_counter, trace_run_length_hist);`
`3917`	`3917`	`return frame;`
`3918`	`3918`	`}`
`3919`	`3919`
Original file line number	Diff line number	Diff line change
`@@ -215,7 +215,7 @@ static void`
`215`	`215`	`print_histogram(FILE out, const char name, uint64_t hist[_Py_UOP_HIST_SIZE])`
`216`	`216`	`{`
`217`	`217`	`for (int i = 0; i < _Py_UOP_HIST_SIZE; i++) {`
`218`		`- fprintf(out, "%s[%d]: %" PRIu64 "\n", name, (1 << (i + _Py_UOP_HIST_BIAS)), hist[i]);`
	`218`	`+ fprintf(out, "%s[%" PRIu64"]: %" PRIu64 "\n", name, (uint64_t)1 << i, hist[i]);`
`219`	`219`	`}`
`220`	`220`	`}`
`221`	`221`