Reduce branches in PyUnicode_MAX_CHAR_VALUE

[kumaraditya303]

Offers a minor 5.5 % speed improvement in microbenchmark but less branches are better especially when it can be done with a small lookup table.

Benchmark:

./python -m timeit -s "a='🚀'" "a*1000"

# Before
2000000 loops, best of 5: 182 nsec per loop
# After
2000000 loops, best of 5: 172 nsec per loop

[kumaraditya303]

I don't understand why msvc is failing to compile.

[kumaraditya303]

MSVC is failing to compile and test_cppext is failing as C++ does not support designated initializers in array by #91321

[gvanrossum]

So just do it the oldfashioned way?

[kumaraditya303]

Done but I wanted to use designated initializer, it looks better

[gvanrossum]

Did you benchmark it on a build with PGO and LTO? Could you also compare before/after for 1-byte wide and 2-byte wide strings? On my Mac (in debug mode, so far) the odd thing is that the 2-byte mode is faster than the 1-byte mode.

I also wonder whether having static data inside a static inline function (that's potentially expanded many times) is a good idea.

[gvanrossum]

Since kind is 3 bits, maybe add extra zero entries for a total of 8?

[gvanrossum]

I do think it's odd that it fails on MSVC, since designated initializers in arrays are supported -- see e.g. Include/internal/pycore_opcode.h. I wonder if MSVC doesn't like having it in a static inline function?

[kumaraditya303]

Well the benchmark is so small that pgo and lto gives inconsistent result because of cpu boost and binary layout. I reject this as this is insignificant and not worth it.

[gvanrossum]

Okay, we learned a few things. :-)

• Use PGO+LTO for benchmarking, even micro-benchmarking.
• Benchmarking is important.
• Maybe a static array inside a static inline function isn't a great idea.
• MSVC seems picky about those.

[kumaraditya303]

Use PGO+LTO for benchmarking, even micro-benchmarking.

Yes, but lto takes a lot of time to compile and can vary in results up to 5 to 10%.

Maybe a static array inside a static inline function isn't a great idea.

Linker would de-dedup those in the executable.

MSVC seems picky about those.

MSVC is picky about designated initializers in array in C++ mode, in C mode it works fine.

[gvanrossum]

Got it!