Refactor Number Protocol BinaryOps & HeapTypeExt - #4720

Because every PyPayload has PyNumberMethods - or even NOT_IMPLEMENTED table, this field can be non-optional.

can this code be simply like this?

        class.slots.as_number().$x

as_number always can exist. when number is not implemented, it can be PyNumberMethods::NOT_IMPLEMENTED.
when this is a python type, it can be a new PyNumberMethods::PYTHON_WRAPPER which holds wrapper functions to call binary ops magic methods.
We don't need to check if it is heaptype or not, becasue PyNumberMethods::PYTHON_WRAPPER will only be set for heaptypes.

The different is as_number will point to a PyNumberMethods witch handles the right_* functions in a same way, but the heaptype hold a PyNumberSlots witch override right_* functions seperated.

The design differ from cpython witch hold both PyNumberMethods on static or heaptype and then as_number is set to point to the right one.

Our design have advantage in our structure is now the PyNumberMethods can be immutable and only use for static, as_number can be immutable and will be override by PyNumberSlots if is heaptype. PyNumberSlots can contains more informations so we can override right_* functions seperately witch in cpython have to fallback both.

So this is like a two atomic read vs one branch, I did not notice the performance change on my own test, but maybe you can help with a better testing.

@youknowone

ah, yep, that was the reason of slot hitting pynumbermethods.

A bit disagreement for the performance advantage. Because we just can add slots to static types too on the top of this patch, I believe as_number can be fully immutable in either way. So it will be one atomic read + one non-atomic read vs one branch + one non-atomic read. So it will be atomic read vs branch problem. And the cost of branch is comparably going more expensive every year. (not sure in this case because branchless design is mostly about arithmetic calculation, not about memory read)

I wonder if either design fits better to fix TestChainMap::test_union_operators regression.

Thinking about this for a while, I'd like to support slot one for its simplicity unless this implementation has recognizable performance gain.

So I think need to close this pr as the memory saving for internal type should not matter. I will split the useful codes to other pr

I thought we only need to edit a little bit more from this PR.
Memory saving for instances means a lot, but not for types. Number of types are limited and especially builtin types are more.

then we also don't need this check
oh, maybe not. because wrapper will always contains function.

this special type checking was added when complex didn't implement AsNumber

complex has AsNumber but did not implement any of int(), index() or float(), so it will fail. This is workaround is also exist in cpython.

I now remember you explained the same thing before 😅