Feature #15095: [PATCH] share VM stack between threads and fibers if identical

• Status: Closed
• Assignee: ko1 (Koichi Sasada)
• Target version: -

[normalperson (Eric Wong)]

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.

[normalperson (Eric Wong)]

Also, why cast in ec_set_vm_stack?
https://80x24.org/spew/[e-mail]/raw
I've barely touched code this week, so maybe I forgot something :x

[ko1 (Koichi Sasada)]

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?

[ko1 (Koichi Sasada)]

normalperson (Eric Wong) wrote:

Also, why cast in ec_set_vm_stack?
https://80x24.org/spew/[e-mail]/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.

[normalperson (Eric Wong)]

[e-mail] 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.

[normalperson (Eric Wong)]

[e-mail] wrote:

normalperson (Eric Wong) wrote:

Also, why cast in ec_set_vm_stack?
https://80x24.org/spew/[e-mail]/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.

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.

[ko1 (Koichi Sasada)]

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

[ko1 (Koichi Sasada)]

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

[normalperson (Eric Wong)]

Koichi Sasada [e-mail] 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.

[normalperson (Eric Wong)]

• 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.

[Feature #15095] [Bug #15050]

v2: vm.c: fix build with USE_THREAD_DATA_RECYCLE==0