-
Notifications
You must be signed in to change notification settings - Fork 289
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add signal support #1026
Add signal support #1026
Conversation
This function let's one register a potential root range from the CAPI Was part of the unmerged pyston#1026 but split out because pyston#1029 requires it too.
30279ab
to
297e25d
Compare
Hey, sorry I didn't get to this before I left. I've lost all the context on this though -- should we close #1024? |
297e25d
to
8bf2bd0
Compare
I closed the old wip PR implementation and rebased this one.
|
lgtm, though it looks like there are some merge conflicts with the frame-introspection change. I'm not sure how much room we have to optimize this other than avoiding the checks -- LLVM produces pretty good code for this (keeps the address of the global in a register), but I think the sheer number of times we add those checks is what leads to the slowdown. |
8bf2bd0
to
dbb09b9
Compare
We check for signals on most calls to runtime functions in the llvm tier and the bjit and inside the interpreter on some additional places, because checking on every bytecode is a too large slowdown (>10%).
dbb09b9
to
28c0c4b
Compare
I fixed the merge conflicts.
I'm still not very happy about this patch but it should to the job until we come up with a nicer solution... |
We check for signals on most calls to runtime functions in the llvm tier and the bjit and inside the interpreter on some additional places, because checking on every bytecode is a too large slowdown (>10%).
I don't like that there is different behavior between the different tiers but it was the best result I could accomplish.
This patch does more checks than pypy does see: https://bitbucket.org/pypy/pypy/issues/1841/signal-check-only-in-jump_absolute-when
I think the perf regression is not nice but I think we could accept it:
I tried generating better code in the bjit (e.g. directly emitting the check or creating a special function which does the pendingCalls check with a special calling convention which does not modify any regs but nothing improved the perf...)
For fun I tried disabling the ticker check which get executed on every bytecode in cpython and saw about a 3.5% perf improvement there...
On additional not nice thing is that this currently does not add checks inside the llvm codegen if the called function is a capi function...