feat: function/closure with keyword arguments#371
feat: function/closure with keyword arguments#371moisespsena wants to merge 6 commits intod5:masterfrom
Conversation
moisespsena
changed the title
geseq
left a comment
geseq
left a comment
There was a problem hiding this comment.
In general you'll have to reduce the amount of type inference in your implementation. I suspect it's also playing a big part in the performance hit you're seeing.
| newArgs := make([]Object, numArgs+len(arr.Value)) | ||
| copy(newArgs, args) | ||
| copy(newArgs[numArgs:], arr.Value) |
vm.go
Outdated
| v.sp-- | ||
| switch arr := v.stack[v.sp].(type) { | ||
| for i := 0; i < numArgs; i++ { | ||
| args = append(args, v.stack[start+i]) |
There was a problem hiding this comment.
This is probably playing a major part. Don't allocate new memory if you can work with existing. If you absolutely need this one create it with the size you need.
vm.go
Outdated
| args []Object | ||
| kwargs, varKwargs map[string]Object |
There was a problem hiding this comment.
try to refactor to get rid of these
There was a problem hiding this comment.
or allocate with the required sizes
vm.go
Outdated
| calleeData := &ImmutableMap{ | ||
| Value: map[string]Object{ | ||
| "fn": callee, | ||
| "args": &ImmutableArray{Value: args}, | ||
| "kwargs": &ImmutableMap{Value: kwargs}, | ||
| }, | ||
| } |
There was a problem hiding this comment.
Also try to see if you can avoid recreating this and update the existing one instead
|
Hopefully these comments will help with a good starting point to optimize the runtime |
|
Hello! What do you think? |
|
Much better but still quite slow. From a cursory glance not sure if anything else could be improved. Perhaps try profiling the benchmark code to see what the pain points are? |
vm.go
Outdated
| hasVarArgs = int(v.curInsts[v.ip+2]) | ||
| numKws = int(v.curInsts[v.ip+3]) | ||
| hasVarKw = int(v.curInsts[v.ip+4]) | ||
| kwargs, varKwargs map[string]Object |
There was a problem hiding this comment.
I believe that the declarations of the variables influence the performance, but are needed. I do not know how to improve this.
var ( // ...
kwargs, varKwargs map[string]Object
)There was a problem hiding this comment.
are you able to allocate it with the required capacity and then manipulate the contents as necessary? that might help
There was a problem hiding this comment.
I removed the variable "Varkwargs" from the main scope, but I had no change in the benchmark.
I defined a capacity for the "kwargs" variable, but it worsened.
Still, I can not find a solution.
2 participants
Hello!
I implement kwargs/keyword arguments for functions and closures. But I'm not very satisfied with the VM performance difference.
Could anyone help me with that?
Original benchmark:
Benchmark after my implementations: