-
Notifications
You must be signed in to change notification settings - Fork 229
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
Preemption [WIP] [RFC] #90
base: master
Are you sure you want to change the base?
Conversation
May I ask you why you'd like to introduce preemption? |
I tried to discuss in Gitter. It might be easier there. |
I'm introducing concurrency to Gravity, with the following semantics:
I/O (sockets) will be represented as processlets that take special commands, and are "owned" by a specific processlet. |
src/cli/gravity.c
Outdated
@@ -158,6 +171,7 @@ int main (int argc, const char* argv[]) { | |||
|
|||
// create VM | |||
gravity_vm *vm = gravity_vm_new(&delegate); | |||
delegate.preempt_callback = &should_preempt; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Formatting
src/runtime/gravity_vm.c
Outdated
|
||
DEBUG_STACK(); | ||
|
||
gravity_preempt_callback preempt_cb = NULL; // Callback to cause preemption |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ditto
src/runtime/gravity_vm.c
Outdated
@@ -1407,7 +1460,7 @@ void gravity_vm_loadclosure (gravity_vm *vm, gravity_closure_t *closure) { | |||
gravity_fiber_reassign(vm->fiber, closure, 0); | |||
|
|||
// execute $moduleinit in order to initialize VM | |||
gravity_vm_exec(vm); | |||
gravity_vm_exec(vm, false); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ditto
src/runtime/gravity_vm.h
Outdated
@@ -21,12 +21,15 @@ typedef void (*vm_transfer_cb) (gravity_vm *vm, gravity_object_t *obj); | |||
typedef void (*vm_cleanup_cb) (gravity_vm *vm); | |||
|
|||
gravity_vm *gravity_vm_new (gravity_delegate_t *delegate); | |||
void gravity_vm_init (gravity_vm *vm, gravity_delegate_t *delegate); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ditto
Have you measured the impact of performing MAYBE_PREEMPT on each instruction? |
It's negligible, performance wise to invoke a callback which returns false. It's about a 3.1% overhead versus commenting out MAYBE_PREEMPT entirely. with preemption callback returning false (median of 50 runs): I think over time, we can introduce lighter weight mechanisms of preemption. Perhaps enabling / disabling it per instruction. |
The other place where need preemption / resumption support is native calls. I have yet to determine the interface here, but when making a call to a native function, it needs to be able to yield up, and resume. I haven't dug in very much, but I was thinking of introducing a return code from native functions, or another function that's bound to them that allows for yielding. |
Why a callback to check if VM must be interrupted and not a simpler: I would image APIs like: bool gravity_vm_interrupt(gravity_vm *vm, gravity_vm_state *state);
bool gravity_vm_resume(gravity_vm *vm, gravity_vm_state *state); In this way no extra parameters are needed and the API is much more cleaner. |
I thought about such an API. It's not a bad approach, I thought about it initially -- especially invoking it from native C calls. I have some questions though:
As a side note, I tried out one other approach -- rather than a run-time callback, doing a compile-time macro. Unfortunately, this had no large speed difference as compared to the callback (<1% faster). |
Something like this should work in my opinion: bool gravity_vm_interrupt (gravity_vm *vm) {
// if vm already interrupted return false
if (vm->interrupted) return false;
// check for mandatory vm_state callback
if (!vm->delegate->vm_state) return false;
// don't think we need to protect this bool flag
vm->interrupted = true;
// wait for vm_state callback to be called
// automatically called by vm when flag is set
return true;
}
void vm_state (gravity_vm *vm, gravity_vm_state *state) {
// this callback is automatically called by the vm when it is interrupted
// state contains info necessary to resume vm (like your vm_state struct)
}
bool gravity_vm_resume(gravity_vm *vm, gravity_vm_state *state) {
// if vm not interrupted return false
if (!vm->interrupted) return false;
// reset interrupted flag
vm->interrupted = false;
// resume execution
gravity_vm_run_resume_main(...);
return true;
}
// your MAYBE_PREEMPT macro is now
#define MAYBE_PREEMPT if (vm->interrupted) goto DO_PREEMPT; |
The ability to be able to interrupt the VM (and step) will open us the possibility to attach a debugger to it. |
If you don't have threads, in which to interrupt the VM, why not look into a yield mechanic, or something similar? Why not a "debug/halt" opcode that halts the VM, also potentially reducing back pressure? |
} | ||
|
||
gravity_vm *gravity_vm_new (gravity_delegate_t *delegate/*, uint32_t context_size, gravity_int_t gcminthreshold, gravity_int_t gcthreshold, gravity_float_t gcratio*/) { | ||
gravity_vm *gravity_vm_new( | ||
gravity_delegate_t *delegate/*, uint32_t context_size, gravity_int_t gcminthreshold, gravity_int_t gcthreshold, gravity_float_t gcratio*/) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Formatting
vm->time += millitime(tstart, tend); | ||
|
||
if (!gravity_vm_preempted(vm)) | ||
PRINT_STATS(vm); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ditto
The callback based method will allow for this as well, specifically, the runtime based callback. |
A couple things:
|
Is the fear of around the callback mechanism about performance? One of the answers here might be to have an interpreter loop with preemption enabled, and one without. You can then run performance-sensitive & trusted code without preemption enabled at all. This is similar to the Linux kernel's preemptive scheduler. |
How about we allow people to define a macro instead of a full-on function. This way, if they want to use the flag, they can have a struct like:
And their macro can look like this:
|
@sargun I'd like to have the ability to decide the pre-emption based on a runtime bool value instead of a macro that would require recompilation. I think that my latest proposal could be the best compromise between performance degradation (a bool comparison on each instruction) and the flexibility to interrupt the vm. For debugging purpose some special bytecode instructions like BREAK and STEP would be the best option. Please let me know what you think. |
What if we do something like have a config option for |
Hi @sargun I am going to re-open it because I'd like to use some of your ideas to implement a built-in debugger. |
Ah sounds good, I was just going through and cleaning out my github activity. |
This PR introduces basic preemption support into Gravity. I'd mostly like to get your thoughts on this. I'd also like to make it so that when (certain) native functions are called, it can also trigger preemption.