Repository navigation
gh-143421: Lazily allocate tracer code and opt buffers - #143597
Fidget-Spinner merged 2 commits into
Conversation
Sorry, something went wrong.
|
Should we remove the newly added pycore_optimizer_types.h? |
Sorry, something went wrong.
Let's keep it. It's a good refactor. |
Sorry, something went wrong.
| // Holds locals, stack, locals, stack ... co_consts (in that order) | ||
| #define MAX_ABSTRACT_INTERP_SIZE 4096 | ||
| // Holds locals, stack, locals, stack ... (in that order) | ||
| #define MAX_ABSTRACT_INTERP_SIZE 512 |
There was a problem hiding this comment.
Why?
Sorry, something went wrong.
There was a problem hiding this comment.
This number was picked arbitrarily by me back then when I first wrote this. I now realise it's overkill. We don't need 3 pages of memory just to store locals and stack. It's slow and wastes memory.
Sorry, something went wrong.
|
This makes no sense to me. How is allocating the necessary memory piecemeal faster than allocating it in a single chunk? |
Sorry, something went wrong.
|
@markshannon no the problem was embedding it as a part of _PyThreadStateImpl, that caused a 100% slowdown of thread spawning time on the benchmark, which reflected in bench_thread_pool. |
Sorry, something went wrong.
|
The benchmark doesn't run any work to use the JIT, instead it just measures thread startup overhead https://github.com/python/pyperformance/blob/main/pyperformance/data-files/benchmarks/bm_concurrent_imap/run_benchmark.py#L19 Embedding the two structs together in the _PyThreadStateImpl causes a massive slowdown. |
Sorry, something went wrong.
|
It shouldn't be embedded, it should be heap allocated as one |
Sorry, something went wrong.
Ok let me just put a up a PR to do that. |
Sorry, something went wrong.
I verified on my system this restores the old performance.