| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
There was a problem hiding this comment.
Looks great!
Sorry, something went wrong.
|
I might have messed something up. Benchmarking came back 2% slower with wonky stats. |
Sorry, something went wrong.
|
I see a few ways to make this faster.
Option 2 sounds a lot simpler, if a bit slower. Unless we go with option 1, we are going to see a lot more PUSH_NULL instructions. It might be worth adding LOAD_FAST_PUSH_NULL and LOAD_CONST_PUSH_NULL superinstructions. My guess is that with option 2, and the the superinstructions, there won't be much of a slowdown. |
Sorry, something went wrong.
|
(And please add a link to the stats/performance numbers) |
Sorry, something went wrong.
|
(The stats issue I observed was in the comparison vs main, and is unrelated.) So, here's the current situation. This branch adds over 7 billion new PUSH_NULL instructions to the benchmark run, which is a significant (~5%) increase in the number of instructions executed. This makes the benchmarks 2% slower (stats). I tried the "easy" route of adding a new LOAD_FAST_PUSH_NULL superinstruction, since the stats suggest that this is by far the most common PUSH_NULL pairing. However, that made it an additional 2% slower (for a total of 4% slower than main), which is surprising (stats). Note that a handful of the slowest benchmarks from this branch got way faster on that branch. So, it seems that clearly something needs to be done, but superinstructions aren't going to help. I'll try some of the other suggested ideas tomorrow, but in general this is getting to be a pretty annoying problem. |
Sorry, something went wrong.
|
That's disappointing; it would seem that moving kwnames to the tstate makes most sense to try next? |
Sorry, something went wrong.
|
Moving keyword names to the frame is a little less than 1% slower vs main. I was getting crashes with moving it to the tstate (which is trickier), but I think I resolved those and I'm benchmarking that approach now. |
Sorry, something went wrong.
|
Sticking it on the tstate is a little more than 1% slower vs main. |
Sorry, something went wrong.
|
Off-line we decided to at least look at splitting CALL into two separate opcode families, CALL and CALL_KW. Looking at the specializations, most of them seem to either insist on no keywords (the majority), or insist on keywords. There are only a few specializations that need to handle either case. So we would not be adding many new opcodes, and the split makes the specialization more efficiently. Using uops we could avoid having more code. Of course, we (Brandt :-) still have to code this and benchmark it before we can be sure of those predictions -- this has been a surprising adventure so far. |
Sorry, something went wrong.
|
Closing in favor of the CALL/CALL_KW split (PR up soon). |
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
This is an updated version of #105849.
📚 Documentation preview 📚: https://cpython-previews--108496.org.readthedocs.build/