Project

General

Profile

Actions

Feature #15095

closed

[PATCH] share VM stack between threads and fibers if identical

Added by normalperson (Eric Wong) over 5 years ago. Updated over 5 years ago.

Status:
Closed
Target version:
-
[ruby-core:88915]

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) over 5 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) over 5 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) over 5 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) over 5 years ago

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) over 5 years ago

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

Updated by ko1 (Koichi Sasada) over 5 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) over 5 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) over 5 years ago

Koichi Sasada 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.

Actions #9

Updated by normalperson (Eric Wong) over 5 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.

[Feature #15095] [Bug #15050]

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

Actions

Also available in: Atom PDF

Like0
Like0Like0Like0Like0Like0Like0Like0Like0Like0