faster-cpython / ideas

1.68k stars 48 forks source link

Lazily fill in `__context__` during unwinding #487

Closed markshannon closed 4 months ago

markshannon commented 1 year ago

When a new Exception object is created, its __context__ is set to the "handled exception" Unfortunately the "handled exception" cannot be determined at compile time or locally, and requires a scan of the call stack.

Rather than perform this expensive check, we could add __context__s during unwinding. The code to do so could be generated by the compiler. We already emit cleanup code in except: blocks, this would be a bit more cleanup. Doing so would speed up EAFP error handling, skipping the expensive step of setting up the __context__. This is semantic change, not just a performance improvement, so will need a wider discussion if we decide it is worthwhile. IMO, this is an improvement to the semantics as the __context__ only makes sense once the scope of that context has been unwound.

Removing the need to scan the stack would also speedup generators and coroutines as we would no longer need to maintain the stack of exceptions to scan.

@iritkatriel Does this make sense to you?

iritkatriel commented 1 year ago

The stack is only scanned to skip over non-Null/Nones, so maybe that can be optimised without changing semantics, simply creating another link to the "next non-null/None exc"?

markshannon commented 1 year ago

That would complicate pushing and popping generator frames, as we would need to check whether the current exception is NULL. It would also require adding the generator frame on entry to a handler, and removing it again on exit. Probably too inefficient, and error prone.

iritkatriel commented 1 year ago

IMO, this is an improvement to the semantics as the __context__ only makes sense once the scope of that context has been unwound.

I agree, and I think this would solve this problem: https://github.com/python/cpython/issues/63061

iritkatriel commented 1 year ago

On the other hand, I don't know how setting it during unwinding would work in this case:

def f():
  raise ValueError(42)

try:
   f()
except:
   try:
      raise TypeError(1)
   except:
      raise TypeError(2)

The outer ValueError is the context of the TypeError(1), but at the time of unwinding we are seeing The ValueError and the TypeError(2) (whose context is TypeError(1), but there could also be a cause messing things up further).

iritkatriel commented 1 year ago

I think it was a mistake to add both __cause__ and __context__ to exceptions. Instead of making the exception chain a tree, it would have been better to have one __chained__ exception + a flag to indicate whether it is the cause or context. I don't know if there are use cases where you really need both. The tree is causing complexity and unsolvable edge cases.

Not sure about the prospects of changing this now.

ericsnowcurrently commented 1 year ago

CC @benjaminp

markshannon commented 4 months ago

I think this is too complex to be worthwhile. Any performance gains would be slight and a simpler approach will get some of the benefit for much less effort.