Using pool to avoid stack and frames memory allocation

[AprilFiv]

#412

[geseq]

I see this being useful for extremely short running scripts (anything that's small enough to run less than double digit microseconds), but the larger a script the less the impact of this as the GC is likely to clean up the pool. Also for runs that take over double digit microseconds the allocation cost should be minimal compared to the runtime itself.

perhaps try your benchmarks with a longer running example like:

fib := func(x) {
	if x == 0 {
		return 0
	} else if x == 1 {
		return 1
	}

	return fib(x-1) + fib(x-2)
}
out := fib(20)

Given that, I'm not entirely convinced that this adds much real value in real world usage.

The code does look safe, but I'm leery against reusing the stack from past runs without any cleanup in future runs and the possibility of that being exploitable. The tradeoff might be worth it if the performance improvement is significant in real world usage, but as it is, I'm not convinced.

The other possibility might be that it's the pointer usage that's giving part of the performance gains. If possible also benchmark this in a long running case to see if there's any difference. If that is partly faster without the usage of pools, it would be a safer bet without having to reuse the same objects.

@d5 thoughts?

[d5]

What happens if one of the runs is aborted by Abort() function? Compiled.RunContext may call VM.Abort if the context is canceled.

[geseq]

Any new run would still start with stack pointer 0 so it should be fine, but the stack itself wouldn’t start out empty. I don’t see any obvious way the items in the stack would be accessible but it’s not ideal.

[AprilFiv]

Any new run would still start with stack pointer 0 so it should be fine, but the stack itself wouldn’t start out empty. I don’t see any obvious way the items in the stack would be accessible but it’s not ideal.

yeah... considered it already

[AprilFiv]

Any new run would still start with stack pointer 0 so it should be fine, but the stack itself wouldn’t start out empty. I don’t see any obvious way the items in the stack would be accessible but it’s not ideal.

By adding such lines, we could clear the stack and frames array literally before recycling, and only cost few extra cpu time, though it makes no sense for VM.Run()

		*v.stack = [len(v.stack)]tengo.Object{}
		*v.frames = [len(v.frames)]frame{}

[geseq]

But does that still provide any gains of the pool?

Do note that the go pool is not like a typical object pool and does get its objects garbage collected if not used frequent enough.

[AprilFiv]

All the pool's target is to reduce the overhead of memory allocation.
These two lines would not make extra memory allocation if you doubt about that.

*v.stack = [len(v.stack)]tengo.Object{}
*v.frames = [len(v.frames)]frame{}

[geseq]

Perhaps I'm not explaining this correctly. My concern is more that if you add up the fact that the pool objects can be GC'd non-deterministically, additional cycles takes for clearing the stack frames, indirect access to stack and frames during the VM run, I'm not sure the gains in memory allocation cost offset enough to make a difference.

Can you please add a benchmark that demonstrates the actual gains using the pool here provides?