hack: try removing error normalization

[davidhewitt]

This is the most minimal diff I could write which removes the storage of "lazy" errors inside PyErr and instead eagerly creates Python exceptions at the point of PyErr creation.

Related to #4584

Now that I push this diff, I see that it's pretty obvious that what this change mainly leads to is that we need a Python<'py> token to create a PyErr (in this PR I do it just internally with Python::with_gil, but maybe this links back to #4413 and we can do that at the same time).

This is not ready to merge yet, I wanted to push it and see what happens in CI...

[codspeed-hq]

CodSpeed Performance Report

Merging #4859 will degrade performances by 87.08%

_{Comparing davidhewitt:no-err-normalization (4d2bf73) with main (ad5f6d4)}

Summary

⚡ 14 improvements
❌ 3 regressions
✅ 67 untouched benchmarks

⚠️ Please fix the performance issues or acknowledge them on CodSpeed.

Benchmarks breakdown

	Benchmark	main	davidhewitt:no-err-normalization	Change
⚡	collect_generic_iterator	105.6 ms	91.9 ms	+14.89%
⚡	ordered_dunder_methods	3.2 µs	2.8 µs	+13.81%
⚡	ordered_richcmp	3.1 µs	2.7 µs	+16.65%
⚡	extract_hashbrown_map	17.9 ms	16.3 ms	+10.13%
❌	err_new_without_gil	1.4 µs	10.5 µs	-87.08%
❌	extract_str_extract_fail	2 µs	10.4 µs	-80.88%
⚡	enum_from_pyobject	24.3 µs	18.1 µs	+33.86%
❌	not_a_list_via_extract	2 µs	10.3 µs	-80.65%
⚡	not_a_list_via_extract_enum	17.9 µs	12.6 µs	+42.16%
⚡	iter_list	6.7 ms	5.9 ms	+15.18%
⚡	list_get_item	4.1 ms	3.3 ms	+24.4%
⚡	list_get_item_unchecked	3.6 ms	3 ms	+21.52%
⚡	iter_tuple	5.9 ms	5.1 ms	+16.41%
⚡	tuple_get_borrowed_item	3.9 ms	3.1 ms	+26.15%
⚡	tuple_get_borrowed_item_unchecked	3.4 ms	2.8 ms	+22.63%
⚡	tuple_get_item	4.1 ms	3.3 ms	+24.39%
⚡	tuple_get_item_unchecked	3.6 ms	2.9 ms	+21.72%

[davidhewitt]

Looks like (so far at least) the tests pass ok.

The codspeed report suggests to me that:

• as above, we probably want to do this change alongside [RFC] split PyErr::new() into multiple methods #4413 so that we make it clear creating a PyErr needs the GIL
• the simplification to error state seems to have a noticeable impact across a lot of whole codebase, it seems desirable from a performance perspective to complete this

[davidhewitt]

The performance regressions seem to come from two cases:

• Creating PyErr without the GIL now blocks on the GIL internally. I'm not worrried about this because I guess that will be a change to the PyErr API (maybe with deprecations).
• Cases where we create a PyErr (from a DowncastError, though could be any Rust error) and then immediately throw it away without checking the actual error content. Previously this would have just made a lazy error, but now it has to create a full Python exception for more wasted work.

I guess this also implies that we probably want to support a user-defined error type for FromPyObject, so that users have an option to avoid PyErr in such cases? (cc @Icxolu)

[Icxolu]

I guess this also implies that we probably want to support a user-defined error type for FromPyObject, so that users have an option to avoid PyErr in such cases?

That looks desirable to me, yes. I believe I have a branch somewhere that introduces FromPyObject::Error: Into<PyErr> that I can revive.

[ngoldbaum]

Awesome that this is a significant net decrease in LOC and hopefully it's rare to create and then discard a PyErr in a non-error code path.

Commenting to subscribe, but please feel free to ping me if you want my help with this.