[Early stage prototype] Optimize calls using vectorcall, 35% reduction in call overhead

[EthanSteinberg]

This PR is an early stage prototype of using Python 3.7's vectorcall support to reduce function call overhead in pybind11.

This is very early stage, it doesn't support all of pybind11's features (kwargs don't quite work) and is in really rough shape code / documentation wise.

I am using a simple benchmark, included in this PR under the folder pybench, to measure performance.

Time to call a C++ function 100000000 times:

Raw CPython API: 5.2974734380841255 seconds
Pybind11 master Branch: 17.06675560772419 seconds
Pybind11 this PR: 11.004007823765278 seconds

I believe I can get this down even further (to around 8 seconds or so) by optimizing the loader_life_support.

The main downsides of this approach:

• We are storing temporary data in function_record objects, so we can't support multiple threads calling the same function_record at a time. This shouldn't matter though as that's always the case due to the GIL

• For older versions of Python (<= 3.6), I will have to implement a fallback that will have a tiny drop in performance due to an extra python tuple and python dict allocation and construction.

What are people's thoughts? Should I fully flush out this PR and add full support for the remaining pybind11 functionality? Do people think this is a useful optimization to add to pybind11?

CC @rwgk

[rwgk]

For older versions of Python (<= 3.6)

3.6 is already ~10 months past EOL (https://endoflife.date/python). This PR is a good motivation to drop it. You could simply remove the corresponding CI jobs in .github/workflows/ci.yml

I only glanced through the code. At first sight it looks like a relatively small change, given what it does. If you want to drive this to completion, I'll look more, run sanitizers & global testing when you get the CI (minus 3.6) healthy here.

Under the smart_holder branch I already introduced a ubench (micro benchmark) directory. Could you please use the same name here, too? Actually, I think it would make sense to copy over the code and strip out the very few lines for smart_holder, to be added back in under the smart_holder branch (I could do that part). You could also remove the python/number_bucket.clif file.

FWIW ubench results from Feb 2021 are here

[Skylion007]

@lalaland If you want to optimize function dispatch, you will want to take a look at this PR too: https://github.com/pybind/pybind11/pull/2013/files/0c8a13d27b7f36dbcaf91833252a5788c0ed429d. It's pretty stale, but it would be great if we could also somehow reduce the number of allocations by using std::array, std::bitset, etc as done in the above PR and get a further cumulative speedup. If that PR was up to date with master, I would gladly merge it along with this one.

One of the reasons@wjakob created https://github.com/wjakob/nanobind/ was to have full supported vectorcall dispatch even if that required the newest versions of Python.

I would definitely onboard with this PR even if we have to drop 3.6 support or if it causes minor binary size increase. We do need some benchmarks that show it's faster (which the linked PR also presented).

[Skylion007]

Lalaland, to try shorten the commit log a bit for this PR, please do the following:

pip install pre-commit
# cd to git project root
pre-commit install

That will run pre-commit locally before every commit.

[EthanSteinberg]

Thanks for the feedback. I'll push this PR to completion. I highly recommend both of you turn off notifications for this PR as I'll probably have to spam the CI quite a bit to get this in a working shape on all platforms.

@Skylion007 I purposely don't run pre-commit to trigger a CI run. I don't have CI permissions, but the precommit bot does so if I can trigger the precommit bot, I get a nice CI run.

[Skylion007]

@Skylion007 I purposely don't run pre-commit to trigger a CI run. I don't have CI permissions, but the precommit bot does so if I can trigger the precommit bot, I get a nice CI run.

@lalaland, Ah, once your other PR is merged that shouldn't be an issue anymore. (You should get permission if you have a merged commit).

[Skylion007]

@lalaland You should now have CI permissions.

[henryiii]

3.8 added vectorcall. That’s why nanobind requires 3.8+. Totally okay to drop 3.6 any time, but I think we’d need to drop 3.7 too to get vectorcall, which I'm not okay with yet.

[Skylion007]

A lot of nits.

As I mentioned before, a lot of the slowdown in master is also from unnecessary memory allocations (std::vector, std::string, etc...) be wary of that as you build out this PR and take a look at the one I linked.

Also be wary of unused template args / such as we are a header only lib so we do want to be careful about slowdown/bloat from unnecessary template instationation. Of course it's fine if those template args are actually used (like for converting an std::vector into an std::array for n_args.

Also you only need the MSVC warning override around stuff that should be constexpr. Unfortunately, there is no easy way to override it in MSVC as the recommended solution is to use constexpr if etc, but that only works if you are > C++14 of course.

[Skylion007]

Suggested change

	constexpr autoalign=std::align;
	usingalign=std::align;

[EthanSteinberg]

This doesn't seem to work? Are you sure this is the correct syntax?

[Skylion007]

Oh sorry, yeah that's the wrong syntax.

Suggested change

	constexpr auto align=std::align;
	usingstd::align;

See

pybind11/include/pybind11/detail/common.h

Line 637 in 09db644

using std::conditional_t;

for example

[Skylion007]

get_child(0) should be cached. It's weird to cache all these members from the child anyway. If you are going to do this instead of caching the child itself, then maybe have a setter that takes in a child object.

[Skylion007]

You could do a C++ version guard with this and do a using alias std::min if C++ >= 14. Some of these details might better level in detail/common.h .

[Skylion007]

Suggested change

	std::stringname;
	std::stringdesc;
	objectvalue; ///< Associated Python object
	boolconvert : 1; ///< True if the argument is allowed to convert when loading
	boolnone : 1; ///< True if None is allowed when loading
	argument_record() : name(""), desc(""), value(), convert(true), none(true) {}
	std::stringname="";
	std::stringdesc="";
	objectvalue(); ///< Associated Python object
	boolconvert : 1= true; ///< True if the argument is allowed to convert when loading
	boolnone : 1= true; ///< True if None is allowed when loading
	argument_record() =default;

[EthanSteinberg]

The bitfields mean that this doesn't work.

warning: default member initializers for bit-fields only available with '-std=c++2a' or '-std=gnu++2a'

[Skylion007]

Wow, these are a lot of template types that are just ignored. Is there a more efficient way to do this that doesn't cause a lot of template initialization? Like making the 2nd arg a void pointer or something?

[Skylion007]

Suggested change

	returnptr=reinterpret_cast<void*>(aligned);
	returnptr=static_cast<void*>(aligned);

[EthanSteinberg]

This isn't support on centos7 for some reason.

[jblespiau]

(FYI, if it's useful, I also extracted and polished https://github.com/jblespiau/pybind11_benchmarks from the nanobind repository, in hope to bring within pybind11 a more extensive benchmarking suite.)

[EthanSteinberg]

Looks like this is very close to v1 being ready. Passes all the tests on regular Python and is quite fast to boot.

The main issue is that I need to do a lot more space optimization

Using @jblespiau's test harness

Time:
python: 0.18168139830231667
nanobind: 0.1628
old pybind11: 0.3840818479657173
this PR: 0.22408869490027428

The problem is space
old pybind11: 1.5625
this PR: 6.1168060302734375

[rwgk]

old pybind11: 1.5625
this PR: 6.1168060302734375

What exactly is 4 times bigger?

Sorry I don't have a lot of experience with micro benchmarks and binary size comparisons.

[EthanSteinberg]

What exactly is 4 times bigger?

The .so files generated when the extensions are complied.

Fixing this should be quite easy. I just need to create non inlined helper functions in several places.

[rwgk]

What exactly is 4 times bigger?

The .so files generated when the extensions are complied.

Wow ... I don't think I've ever seen such an explosion of binary sizes.

I just need to create non inlined helper functions in several places.

Hm ... I did measurements back in Aug 2021:

https://docs.google.com/spreadsheets/d/1bkg1HtmU1AjotPTXCCLSbTBGARRet8W91noh1DnKOHU/edit#gid=0

noinline hardly made a difference, but maybe your situation is somehow special?

[EthanSteinberg]

noinline hardly made a difference, but maybe your situation is somehow special?

The situation here is special because of the templates involved. So pulling an even small amount of code out of the template (and forcing it out of the template with non-inline) can have huge savings because of the hundreds of copies.

Wow ... I don't think I've ever seen such an explosion of binary sizes.

Keep in mind that these microbenchmarks are sorta contrived. In order to make the sizes / compile times more significant the same code is copied 719 times.

[Skylion007]

noinline hardly made a difference, but maybe your situation is somehow special?

The situation here is special because of the templates involved. So pulling an even small amount of code out of the template (and forcing it out of the template with non-inline) can have huge savings because of the hundreds of copies.

Wow ... I don't think I've ever seen such an explosion of binary sizes.

Keep in mind that these microbenchmarks are sorta contrived. In order to make the sizes / compile times more significant the same code is copied 719 times.

This was also the case in the PR I linked here: #2013 they were eventually able to resolve it.

[henryiii]

Python 3.6 now dropped! Planning on dropping 3.7 immediately after the next release, too. That should make this much easier.