migrate many FromPyObject implementations to Bound API

[davidhewitt]

Split from #3606

This is a series of mechanical transformations from extract to extract_bound for most FromPyObject implementations across the codebase.

The few which remain are mostly blocked on other PRs waiting for review, it'll be straightforward to follow up with those another time.

[codspeed-hq]

CodSpeed Performance Report

Merging #3784 will degrade performances by 14.16%

_{Comparing davidhewitt:more-extract-bound (0d4df9c) with main (975f182)}

Summary

⚡ 16 improvements
❌ 2 regressions
✅ 61 untouched benchmarks

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

Benchmarks breakdown

	Benchmark	main	davidhewitt:more-extract-bound	Change
⚡	list_get_item	20.1 ms	16.2 ms	+23.68%
⚡	iter_list	31.6 ms	23.8 ms	+33.03%
⚡	sequence_from_list	425 ns	300 ns	+41.67%
⚡	list_get_item_unchecked	17.1 ms	13.1 ms	+29.88%
⚡	extract_hashmap	94 ms	85.1 ms	+10.47%
⚡	mapping_from_dict	447.8 ns	272.2 ns	+64.49%
⚡	not_a_list_via_downcast	270.6 ns	215 ns	+25.84%
❌	test_args_kwargs_rs	26.8 µs	30 µs	-10.74%
⚡	extract_btreeset	73.7 ms	65.5 ms	+12.56%
⚡	tuple_get_item	20.1 ms	16.2 ms	+23.68%
❌	list_via_extract	336.7 ns	392.2 ns	-14.16%
⚡	iter_tuple	27.1 ms	18.9 ms	+43.22%
⚡	tuple_get_borrowed_item_unchecked	15.7 ms	11.6 ms	+35.16%
⚡	iter_set	54.1 ms	41.4 ms	+30.59%
⚡	sequence_from_tuple	397.2 ns	226.1 ns	+75.68%
⚡	tuple_get_borrowed_item	18.9 ms	14.9 ms	+26.92%
⚡	tuple_get_item_unchecked	16.9 ms	13 ms	+30.26%
⚡	f64_from_pyobject	822.2 ns	461.1 ns	+78.31%

[davidhewitt]

Apart from the known volatility on the list_via_downcast benchmark, which is unrelated to this PR and would be nice to get to the bottom of sometime, the large set of performance wins that we see here is pretty consistent again with what I normally see by replacing uses of the GIL Refs API with the new Bound API :)

[Icxolu]

While we're here, it might be nice to rename all the named lifetimes ('source, 's, ...) to 'py for clarity and consistency with the rest of the code base. This would also easy the transition to a future with a second lifetime in FromPyObject.

Otherwise this looks good to me.

[davidhewitt]

Thanks for the review; and yes, that's a great idea, I'll push that now as a separate commit.

Have you got access to the approval button when reviewing? If not, perhaps I can look into setting that up and also labelling so you can also apply the CI-skip-changelog label...

[Icxolu]

I think approving might work, I just was not sure if you want me to do that. I dont think I can edit the labels.

[adamreichold]

Was not moving this to extract_bound intentional?

[davidhewitt]

Yes, there's a bit of complexity around Cow, &str and &[u8] because they need to depend on the lifetime of the input &Bound.

For 0.21 I don't want to touch them, they will just have to continue with the pool I think. In 0.22 or 0.23 we may have to remove their FromPyObject implementations and implement PyFunctionArgument for them. Or we keep the pool for a little longer as an internal only thing just for these three implementations.

The right long term solution will again be adding the second lifetime to FromPyObject so we can implement these implementations correctly.

[adamreichold]

Uff, so this means that 0.21 will not be fully compatible with gevent then?

I wonder if having something like Backed<T> where T is separate Rust type with inline storage (e.g. &[u8]), but backed by a Python pointer would be able to solve this without adding a lifetime to the schema at the cost of a reference count bump.

[davidhewitt]

I've wondered a little about some kind of Backed<T> type too, but I haven't explored the practicalities of how that would actually be implemented. I'd marked that as a future feature in my head.

Uff, so this means that 0.21 will not be fully compatible with gevent then?

One option is to gate these FromPyObject implementations on the gil-refs feature. That would be a normal "additive" feature. Without the feature enabled we implement PyFunctionArgument instead. I will add that as a bullet in the tracking issue and potentially will push it as a draft later.

(At least then if users follow our migration steps their code wouldn't break from this until they disable the gil-refs feature.)

[adamreichold]

Similar to that for Cow. (The Cow one should maybe also move here?)

[davidhewitt]

Sadly yes, similar to the above. I'll move the implementation here though 👍