-
Notifications
You must be signed in to change notification settings - Fork 307
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
Introduce builtin function govm and makechan #330
base: master
Are you sure you want to change the base?
Conversation
Builtin go(fn, arg1, arg2, ...) starts a goroutine which run fn(arg1, arg2, ...) in a new VM cloned from the current running VM, and returns a job object that has wait, result, abort methods. Builtin makechan(size) accepts an optional size parameter, makes a channel to send/receive object, and returns a chan object that has send, recv, close methods. To be able to access *VM instance that it is running in for go() and makechan(), now VM will always pass VMObj to and only to BuiltinFunction as arg[0].
A goroutineVM is a VM cloned from the current VM in a shared way that the cloned VM(the child) can access globals and constants of the parent VM, but with its own stack and frames. The cloned VM will be running with a specified compiled function as the entrypoint in a new goroutine created by the parent VM.
In golang spec, "go" statement has no return value, which is not the semantic here: we want "govm" returns a goroutineVM object which can be used to wait, abort, or get result of the goroutineVM.
Hello! Thanks for opening a PR. I really like the idea of adding some concurrency constructs to Tengo. I think this is a good start. But, there's one thing that I have to mention before going into more details. It seems that this change can significantly slow down the running performance of the regular non-concurrent code. (E.g. The Fibonacci benchmark is ~10% slower.) Also, I personally prefer to keep Tengo as minimal and simple as possible. Concurrency can significantly increase the maintenance costs overall, which we cannot afford at the moment. |
Hi d5,
I will dig deeper of the Fibonacci test to understand why... |
VM will only pass its vmObj to builtin function that has needvmObj equals true, so that normal builtin functions do not need to notice the change.
I have run the Fibonacci bench in cmd/bench several times manually, but the result varies from each run, from master branch I got:
And with this feature branch, on the same X86 PC, I got:
I know it is just a manual run, but from the result, it looks to me more about the result variation of the same binary in different runs... I will try to add more concurrency test to verify this goroutineVM feature. |
The goroutineVM will not exit unless: 1. All its descendant VMs exit 2. It calls abort() 3. Its goroutineVM object abort() is called on behalf of its parent VM The latter 2 cases will trigger aborting procedure of all the descendant VMs, which will further result in d5#1 above.
As far as I can tell the fib benchmark itself shouldn’t be affected as there’s nothing in the execution path within the vm that has changed. I ran it a few times and it’s mostly inconclusive. |
While I agree that this significant maintenance cost, I think adding concurrency constructs might be worth it. |
Take below snipet as an example: $ cat cmd/tengo/test.tengo test1 := func() { v := 1 test2 := func() { len(1,1,1,1) v++ } test2() } test1() ---- before the fix: $ ./tengo test.tengo Runtime Error: wrong number of arguments in call to 'builtin-function:len' at - at test.tengo:7:2 at test.tengo:10:1 ---- after the fix: $ ./tengo test.tengo Runtime Error: wrong number of arguments in call to 'builtin-function:len' at test.tengo:4:3 at test.tengo:7:2 at test.tengo:10:1
with this commit: 1. builtinGovm() now accepts native golang function, but unlike compiled functions which run in a cloned VM, native golang functions do not need VM, they just run in a goroutine. 2. Parent VMs now have records of errors of all its descendent VMs. A VM's error is a combination of the error of itself and all errors of the descendent VMs, in another word, parent VM's error includes all errors of the child VMs. 3. ErrVMAborted is no longer treated as runtime error.
now that "govm" does not always run function in VM, so "go" is better. update docs to reflect the usage changes.
Thanks @geseq |
And now abort() on the same VM twice will not panic.
On a panic happens in a VM: * the panic value along with both its script level and native level callstack are recorded as run time error, its parent VM should examine this error. * the panic will not cause whole program exits, but it does trigger the abort chain, so that the VM in which the panic happens and all its descendent VMs will terminate. So panic only causes the descendent group of VMs to exit. Other VM groups are not affected.
Export VMObj to std.fmt, so that fmtPrintln can get vm.Out to output to a io.Writer per VM.
and nolonger hide ErrVMAborted
What about this PR. I think it could afford my needs: #335 |
and nil child VM to gc after it is done
This reduce memory foot print significantly when there are large number of small VMs running.
@jvegaseg I think @Bai-Yingjie is still working on improving the stability of this. We’ll re-evaluate once it’s ready. |
Hi,
I've added 2 built-in functions: go and makechan, to run multiple VMs in a similar way that native Golang's go and channel do.
go(fn, arg1, arg2, ...) starts an independent concurrent goroutine which runs fn(arg1, arg2, ...)
If fn is CompiledFunction, the current running VM will be cloned to create a new VM in which the CompiledFunction will be running.
The fn can also be any object that has Call() method, such as BuiltinFunction, in which case no cloned VM will be created.
Returns a goroutineVM object that has wait, result, abort methods.
The goroutineVM will not exit unless:
The latter 2 cases will trigger aborting procedure of all the descendant
goroutineVMs, which will further result in README #1 above.
wait()
waits for the goroutineVM to complete up to timeout seconds andreturns true if the goroutineVM exited(successfully or not) within the
timeout. It waits forever if the optional timeout not specified,
or timeout < 0.
abort()
triggers the termination process of the goroutineVM and allits descendant VMs.
result()
waits the goroutineVM to complete, returns Error object ifany runtime error occurred during the execution, otherwise returns the
result value of fn(arg1, arg2, ...)
More docs:
https://github.com/godevsig/tengo/blob/dev/docs/builtins.md#go
How do you think about it? I am new here, but I love tengo so much, any comments/advices are welcome~