Add benchmarks for ctypes function call overhead - #197
mdboom wants to merge 2 commits into
Conversation
Sorry, something went wrong.
|
Wanted to add a note: This is definitely a "microbenchmark" that isn't a great example of a real-world workload. |
Sorry, something went wrong.
ericsnowcurrently
left a comment
There was a problem hiding this comment.
Mostly LGTM. There are some slight changes we should consider though, particularly using runner.bench_time_func() for micro benchmarks.
Sorry, something went wrong.
| void_foo_void() | ||
| int_foo_int(1) | ||
| void_foo_int(1) | ||
| void_foo_int_int(1, 2) | ||
| void_foo_int_int_int(1, 2, 3) | ||
| void_foo_int_int_int_int(1, 2, 3, 4) | ||
| void_foo_constchar(b"bytes") |
There was a problem hiding this comment.
The real benefit of micro-benchmarks is that it narrows down where performance regressions might be. With that in mind, would these different signatures have enough independent potential for regression that it would it make sense to have a separate benchmark for each? Would it be worth bothering even if they did?
Sorry, something went wrong.
ericsnowcurrently
left a comment
There was a problem hiding this comment.
Thanks for the changes. I just have a couple more suggestions.
Sorry, something went wrong.
|
LGTM, but I am a bit concerned that benchmarking all of the call argtypes in one iteration can hide regressions, but I can't think of anyway to fix that apart from splitting this benchmark further to each call argtypes. |
Sorry, something went wrong.
ericsnowcurrently
left a comment
There was a problem hiding this comment.
Mostly LGTM. I've left a few comments on a few minor things and to get clarification in a couple spots.
Sorry, something went wrong.
ericsnowcurrently
left a comment
There was a problem hiding this comment.
A quick follow-up suggestion...
Sorry, something went wrong.
ericsnowcurrently
left a comment
There was a problem hiding this comment.
LGTM
I'll leave it to you about my recommended change.
Sorry, something went wrong.
ericsnowcurrently
left a comment
There was a problem hiding this comment.
LGTM
There's only one small suggestion, which I'll leave to your discretion.
Sorry, something went wrong.
| if os.path.isfile(req): | ||
| name = os.path.basename(req) | ||
| if name == "setup.py": | ||
| req = os.path.dirname(req) |
There was a problem hiding this comment.
The reason we use the dirname isn't obvious, so it may be worth adding a comment here indicating pip's limitations.
Sorry, something went wrong.
| # pip doesn't support installing a setup.py, | ||
| # but it does support installing from the directory it is in. |
There was a problem hiding this comment.
This comment is what I was thinking of above. Consider moving it there.
Sorry, something went wrong.
Add ctypes and ctypes_argtypes benchmarks
This additionally adds support for building C extensions as part of creating a benchmark's virtual environment, since that didn't seem to be possible prior to this change.
See faster-cpython/ideas#370 for some background discussion.