deprecate PyDict::new constructor

[Icxolu]

Part of #3684, followup on #3817 and last piece of #3716.

This deprecates the deprecates the PyDict::new constructor. In the second commit I also updated the benchmarks to make use of the new APIs.

[codspeed-hq]

CodSpeed Performance Report

Merging #3823 will degrade performances by 14.16%

_{Comparing Icxolu:pydict (ad00779) with main (c56cd3d)}

Summary

⚡ 1 improvements
❌ 4 regressions
✅ 74 untouched benchmarks

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

Benchmarks breakdown

	Benchmark	main	Icxolu:pydict	Change
⚡	iter_dict	49.6 ms	37.7 ms	+31.47%
❌	extract_str_downcast_fail	269.4 ns	300.6 ns	-10.35%
❌	extract_str_extract_fail	2.4 µs	2.7 µs	-13.31%
❌	list_via_extract	336.7 ns	392.2 ns	-14.16%
❌	extract_int_downcast_fail	269.4 ns	300.6 ns	-10.35%

[davidhewitt]

Thanks yet again!

Just a couple of small things I'd like to see tweaked on this one. As per #3811 I've just enabled squash merges, so up to you if you want to force-push the update or just add a new commit on top. 👍

Have you got further pieces that you're interested in picking up next / do you want suggestions?

[davidhewitt]

To keep the bechmark comparable, it would probably make sense to use iter_with_large_drop:

Suggested change

	b.iter(|| (0..LEN as u64).map(|i| (i, i * 2)).into_py_dict_bound(py));
	b.iter_with_large_drop(|| (0..LEN as u64).map(|i| (i, i * 2)).into_py_dict_bound(py));

Motivation: the GIL Ref &PyDict will live until the with_gil closure ends, so the way the benchmark has changed means that we're now measuring freeing the dict when we weren't before.

[Icxolu]

Makes sense 👍

[davidhewitt]

I think generally I prefer the turbofish style here, as it'll be less residual work when we tidy up FromPyObject in the future:

Suggested change

	let f = Foo::extract_bound(dict.as_ref()).expect("Failed to extract Foo from dict");
	let f = dict.extract::<Foo>().expect("Failed to extract Foo from dict");

and similar for a lot of other cases in this file.

[Icxolu]

I've updated the ones related to my changes here (or would you like to also include the ones not relevant to the PyDict conversion?)

[davidhewitt]

Ah, let's just change them all? Sorry I missed these before 🤦

[Icxolu]

I've added these as a separate commit

[Icxolu]

Have you got further pieces that you're interested in picking up next / do you want suggestions?

I'm open for suggestions 😃

[davidhewitt]

I'm open for suggestions 😃

Awesome 🚀 ! So here's a couple ideas:

• If you're in the flow for more of the same kind of thing, I think Python::import and Python::get_type need migrating and would probably both be worth PRs in their own right and probably help get a lot of stuff off the GIL Refs. Both of them are a bit related to the module methods and type methods PRs that I got stuck in draft 🙈. I think it would be fine to make the implementation of Python::import_bound something like PyModule::import("foo").as_borrowed().to_owned() for now and that would help things move forward a lot.
• If you want to instead go into the proc-macro wilderness, #[derive(FromPyObject)] needs updating to generate an extract_bound instead of an extract. I haven't even tried to look at that 😂

[davidhewitt]

💯 perfect, thanks!

[Icxolu]

Thanks for the suggestions!

I think I will start with looking at the FromPyObject proc-macro. I haven't seen much of the macro stuff so far, so this sounds like fun.

[davidhewitt]

Great! I expect the two main files to dig around in will be src/impl_/frompyobject.rs and pyo3-macros-backend/src/frompyobject.rs