Improve Parser/token.c to be more branch predictor friendly and easier to read.

[DefinitelyNotAnOrca]

Feature or enhancement

Proposal:

I was digging through the python parser, and noticed the file Parser/token.c was pretty hard to follow because it had a series of nested switch statements that looked like:

int
_PyToken_TwoChars(int c1, int c2)
{
    switch (c1) {
    case '!':
        switch (c2) {
        case '=': return NOTEQUAL;
        }
        break;
    case '%':
        switch (c2) {
        case '=': return PERCENTEQUAL;
        }
        break;

this type of nested switching leads to branch predictors not being able to easily guess the branch paths (and is harder to read).

I am proposing to change _PyToken_TwoChars and _PyToken_ThreeChars to use a single switch statement with a C macro to bind the switches into single statements.

Before:

int
_PyToken_TwoChars(int c1, int c2)
{
    switch (c1) {
    case '!':
        switch (c2) {
        case '=': return NOTEQUAL;
        }
        break;
    case '%':
        switch (c2) {
        case '=': return PERCENTEQUAL;
        }
        break;

After:

int
_PyToken_TwoChars(int c1, int c2)
{
    switch (GENERATE_2CHAR_CODE(c1, c2)) {
        case GENERATE_2CHAR_CODE('!', '='): return NOTEQUAL;
        case GENERATE_2CHAR_CODE('%', '='): return PERCENTEQUAL;
        case GENERATE_2CHAR_CODE('&', '='): return AMPEREQUAL;

Has this already been discussed elsewhere?

This is a minor feature, which does not need previous discussion elsewhere

Links to previous discussion of this feature:

No response

Linked PRs

• gh-126049: Updated Parser/Token.c to unnest switch statements #126050

[picnixz]

How are performances affected if you change it like this? could you benchmark the change using pyperf please?

[DefinitelyNotAnOrca]

GCP_RESULTS.zip The performance changes are very negligible (to the point that I think they are smaller than the background noise on my GCP VM

[DefinitelyNotAnOrca]

The largest difference at this point is readability, and it changes the token.o binary size from 16K to 12K (testing on an AMD64 Platform)

[DefinitelyNotAnOrca]

Oh, I did my bench marking with pyperformance, and not pyperf, let me get those added

[JelleZijlstra]

Given that this is generated code and your PR makes the code generator more complex not less, I don't see a reason to accept this change unless it measurably improves performance.

It's even possible that your change doesn't actually affect the machine code generated by the C compiler.

[picnixz]

Oh, I did my bench marking with pyperformance, and not pyperf, let me get those added

I think it doesn't really matter because they both do more or less the same. I was just interested in knowing whether the parser is faster or not :') (pyperf allows you to report nice timings without having to attach a file)

By the way, we weren't saying that your issue should be closed, just that we would consider it if it improves performances! hence, there was no need to close it! (If you want to abandon this proposal, please tell me; we'll close this one as "not planned" instead [that's just for triaging])

[picnixz]

Closing as not planned since the PR was abandonned.