pystats specialization deferred numbers are wrong

[mdboom]

Bug report

Bug description:

For many of the "deferred" values in the "specialization" section for each bytecode, the number is enormous. For example

Kind	Count	Ratio
deferred	737,869,762,948,500,785,112	53,482,007,414,484.3%
hit	1,120,068,160	81.2%
miss	138,055,084	10.0%

We need to investigate where this is coming from (either the Python runtime or the summarize_stats.py script) and fix appropriately.

CPython versions tested on:

CPython main branch

Operating systems tested on:

Linux

Linked PRs

• gh-113010: Don't decrement deferred in pystats #113032

[mdboom]

The source of the issue seems to be decrementing the deferred counter more times than it's incremented, thus going negative. (The larger than 64 bit number is created because it's an aggregate of multiple runs, from the summarize_stats.py script which uses variable length integer arithmetic).

The deferred value is decremented inside of DEOPT_IF (inside UPDATE_MISS_STATS), with a comment that it's to counteract an erroneous increment elsewhere, but maybe that no longer happens? @markshannon: Does that make sense to you given recent changes? Maybe we just remove the STAT_DEC?

#ifdef Py_STATS
#define UPDATE_MISS_STATS(INSTNAME)                              \
    do {                                                         \
        STAT_INC(opcode, miss);                                  \
        STAT_INC((INSTNAME), miss);                              \
        /* The counter is always the first cache entry: */       \
        if (ADAPTIVE_COUNTER_IS_ZERO(next_instr->cache)) {       \
            STAT_INC((INSTNAME), deopt);                         \
        }                                                        \
        else {                                                   \
            /* This is about to be (incorrectly) incremented: */ \
            STAT_DEC((INSTNAME), deferred);                      \
        }                                                        \
    } while (0)
#else
#define UPDATE_MISS_STATS(INSTNAME) ((void)0)
#endif

https://github.com/python/cpython/blob/main/Python/ceval_macros.h#L263

[markshannon]

Yes, it looks like we specifically emit the INSTRUCTION_STATS macro before the label so that it isn't counted twice.

Do we actually need the deferred stats at all? It doesn't look we increment them anywhere.
I think they date from the days when we had a distinct "base" and "deferred" version of instructions.

[mdboom]

We do increment deferred from a number of bytecodes in bytecodes.c, for example: https://github.com/python/cpython/blob/main/Python/bytecodes.c#L328

However, I can't speak to whether they are still needed. It's easy enough to remove the whole field.

I'm also re-running pystats now with the decrement removed so we can see whether the results look plausible.

[hugovk]

Triage: the linked PR is merged, can this be closed?

[hugovk]

Triage: the linked PR is merged, can this be closed?

Assuming yes, please re-open if still needed 👍