Hello, and thanks for your contribution!

I'm a bot set up to make sure that the project can legally accept this contribution by verifying everyone involved has signed the PSF contributor agreement (CLA).

CLA Missing

Our records indicate the following people have not signed the CLA:

@kornicameister

For legal reasons we need all the people listed to sign the CLA before we can look at your contribution. Please follow the steps outlined in the CPython devguide to rectify this issue.

If you have recently signed the CLA, please wait at least one business day
before our records are updated.

You can check yourself to see if the CLA has been received.

Thanks again for the contribution, we look forward to reviewing it!

ok, haven't exactly read entire contributing docs and missed News bit, I will deal with that shortly.
I am more worried about lack of CLA which I have signed just yesterday :/

just signed CLA again, changed bugs.python.org username to match GH account (not even sure if that was needed, better be sorry though). Not sure if there's anything else I can now, I thought I've completed process correctly.

@gvanrossum alright, so let's wait for CLA to be processed. I assume my job here is done and bot will simply change label if CLA gets in.

cheers 👍

I am a bit worried about performance impact. This is relevant only for Literal[...] but will affect all generics like List[int]. If the performance impact is small, then it is OK, otherwise I would specifically used the typed cache version only for Literal[...].

(Removed the auto-merge label until perf question is resolved).

@ilevkivskyi seems a bit contradicting ;), performance issue and caching. But that's worth checking anyway, I will do such comparison later on.

What is contradicting here? Typed cache lookup will be slower (because we need to compare the types), the question is how much slower.

Looking at the source, the difference in the cache is that with typed=False, the cache key used for Literal[1] will be the integer object 1, while with typed=True it will construct a tuple (1, int). Or, more likely, since @_tp_cache is applied to a method (__getitem__), the cache key without will be the tuple (self, 1) and the cache key with will be (self, 1, Literal, int). So cache key construction will be a bit slower and cache lookup will too. Using timeit it might be easy for the OP to check whether this is significant. (Note that the "real" code is written in C, and I didn't look at it, but IIRC it faithfully follows the Python version linked above.)

Results:

Scripts:

import typing as t
import timeit as tt
import time
import operator
import functools

stmts = (
    't.Literal[1]',
    't.Literal[1]',
    '(t.Literal[1], t.Literal[1], t.Literal[0], t.Literal[0])',
    't.Literal[True]',
    't.Literal[True, False]',
    '(t.Literal[True, False], t.Literal[True, False], t.Literal[True, False], t.Literal[True, False], t.Literal[True, False])',
    't.Literal[0]',
    't.Literal[0, 1]',
    't.List[t.Literal[1,2,3]]',
    't.Union[float, complex]',
    't.Union[str, bytes, float, complex, t.Literal[1]]',
    't.NewType("NT", int)',
    't.Literal[1,2,3,4,5,6,7,8]',
    't.Dict[t.AnyStr, complex]',
    't.Set[t.AnyStr]',
    't.Set[t.Union[float, int]]',
    't.AbstractSet[t.Union[float, int]]',
    't.Sequence[t.Union[float, int]]',
    't.NamedTuple("A", x=int)',
    't.NamedTuple("B", x=int,y=float)',
    't.NamedTuple("C", x=int,y=float,z=complex)',
)
repeats = 300.0
exec_number = 1000

for stmt in stmts:
    times=tt.repeat(
        stmt,
        globals={
            't': t,
        },
        number=exec_number,
        repeat=int(repeats),
        timer=time.perf_counter,
    )
    avg = functools.reduce(operator.add, times) / repeats
    print(f'{stmt} => {avg*1000.0:.5f}ms')

indeed, having typed=False results in performance impact but worst results can be observed when there are two caches one with typed=True and the other with typed=False i.e.:

normal_cache = functools.lru_cache(typed=False)(func)
literal_cached = functools.lru_cache(typed=True)(func)

_cleanups.append(normal_cache.cache_clear)
_cleanups.append(literal_cached.cache_clear)

a bit worried about performance impact. This is relevant only for Literal[...] but will affect all generics like List[int]. If the performance impact is small, then it is OK, otherwise I would specifically used the typed cache version only for Literal[...].

@ilevkivskyi mentioned about having cache typed=True just for Literal.

I was hoping that maybe there will be a possibility to affect __eq__ or __hash__ for Literal but given how it is constructed, I don't see how it can be achieved at reasonable cost.

TBH, this doesn't make much sense. If you only change the behavior of the cache for Literal[...], then how this can affect the performance of existing unchanged code for List[int]? You likely just see the noise (btw there is no variance in your table).

perhaps the way I've done that affected results, I've simply put a ternary operator inside of function that is wrapped in _tp_cache to have something like: return literal_cached(*args,**kwargs) if args[0] is Literal else cached(*args, **kwargs**). @ilevkivskyi was that what you had in mind? Or was that something like creating another function, similar to _tp_cache like _literal_tp_cache that uses typed cache?

No, you should remove the Literal branch at the end of _SpecialForm.__getitem__(), override __getitem__() (using the removed branch and typed cache) in a subclass (lets say _TypedSpecialForm or _LiteralSpecialForm), and make Literal an instance of the latter.

@ilevkivskyi thx for that tip, that will save us some time during reviewing 👍
Before a patch is posted, a little heads up. I've been trying to utilize existing _tp_cache function and just make it aware of typed: bool argument that defaults to False. Following error is what stopped me from doing just that:

function() argument 1 must be code, not str.

I couldn't track where does it come from so decided to just make two decorators one for typed=True and the other for typed=False.

BTW: Decided to took _TypedSpecialForm name