Feature #15095
closed[PATCH] share VM stack between threads and fibers if identical
Description
ec->vm_stack is always allocated with malloc, so stacks for root
fiber (thread stack) and non-root fibers can be shared as long
as the size is the same. The purpose of this change was mainly
to reduce dependencies on ROOT_FIBER_CONTEXT, but it makes sense
to share reusable data as much as possible, regardless.
Will commit in a day or two (part of plan for [Bug #15050]).
Long term, I think it makes sense to get rid of distinction
between "thread" and "fiber" stacks, because every thread
has a (root) fiber, anyways.
Files
Updated by normalperson (Eric Wong) about 6 years ago
Also, why cast in ec_set_vm_stack?
https://80x24.org/spew/20180909080140.16293-1-e@80x24.org/raw
I've barely touched code this week, so maybe I forgot something :x
Updated by ko1 (Koichi Sasada) about 6 years ago
ec->vm_stack is always allocated with malloc, so stacks for root
fiber (thread stack) and non-root fibers can be shared as long
as the size is the same.
I'm not sure why we can share the VM stack. If we run root fiber and non-root fiber with switching, it will conflicts.
Do I misunderstand something?
Updated by ko1 (Koichi Sasada) about 6 years ago
normalperson (Eric Wong) wrote:
Also, why cast in ec_set_vm_stack?
https://80x24.org/spew/20180909080140.16293-1-e@80x24.org/raw
I've barely touched code this week, so maybe I forgot something :x
Yes. These fields were const
at least on my laptop to modify something.
Updated by normalperson (Eric Wong) about 6 years ago
ko1@atdot.net wrote:
ec->vm_stack is always allocated with malloc, so stacks for root
fiber (thread stack) and non-root fibers can be shared as long
as the size is the same.I'm not sure why we can share the VM stack. If we run root fiber and non-root fiber with switching, it will conflicts.
Do I misunderstand something?
Sorry, I mean they can share VM stack cache; not the in-use VM stack.
Updated by normalperson (Eric Wong) about 6 years ago
ko1@atdot.net wrote:
normalperson (Eric Wong) wrote:
Also, why cast in ec_set_vm_stack?
https://80x24.org/spew/20180909080140.16293-1-e@80x24.org/raw
I've barely touched code this week, so maybe I forgot something :xYes. These fields were
const
at least on my laptop to modify something.
OK, I thought that was the case. But no plans to use `const' here
in the future, right? In other words, this patch should be OK?
Thanks.
Updated by ko1 (Koichi Sasada) about 6 years ago
On 2018/09/10 2:31, Eric Wong wrote:
OK, I thought that was the case. But no plans to use `const' here
in the future, right? In other words, this patch should be OK?
Not sure about the future.
But it is okay to commit your patch.
--
// SASADA Koichi at atdot dot net
Updated by ko1 (Koichi Sasada) about 6 years ago
On 2018/09/10 2:26, Eric Wong wrote:
Sorry, I mean they can share VM stack cache; not the in-use VM stack.
It makes sense.
But I remember that Fiber stack is smaller than Thread VM stack because
of rough estimation (Fiber may not require a big stack). Does it okay?
--
// SASADA Koichi at atdot dot net
Updated by normalperson (Eric Wong) about 6 years ago
Koichi Sasada ko1@atdot.net wrote:
On 2018/09/10 2:26, Eric Wong wrote:
Sorry, I mean they can share VM stack cache; not the in-use VM stack.
It makes sense.
But I remember that Fiber stack is smaller than Thread VM stack because of
rough estimation (Fiber may not require a big stack). Does it okay?
Yes, cache will only be reused if fiber and thread VM stack size
match. Maybe most users don't set them to the same value, but I
do when using fibers.
In the future, maybe we can make them the same in config to
minimize needless difference and simplify configs/documentation.
It will be a separate proposal.
Updated by normalperson (Eric Wong) about 6 years ago
- Status changed from Open to Closed
Applied in changeset trunk|r64703.
share VM stack between threads and fibers if identical in size
ec->vm_stack is always allocated with malloc, so stack cache for
root fiber (thread stack) and non-root fibers can be shared as
long as the size is the same. The purpose of this change is to
reduce dependencies on ROOT_FIBER_CONTEXT.
v2: vm.c: fix build with USE_THREAD_DATA_RECYCLE==0