Feature #10341: Fiber switch performance improvements

• Status: Closed
• Assignee: -
• Target version: -

[nome (Knut Franke)]

The attached patches increase performance of switches between Fibers by ~17% on my test system (Linux, gcc 4.8.2).

Patches 1-3 are purely cosmetic, but included here because submitting them separately would cause conflicts between the patch sets. If these are rejected, I can prepare stand-alone versions of 4/5.

Patch 4 yields the most significant performance increase (~12%). The benefit of patch 5 is lower, and it's a larger change; so this one could optionally be omitted.

require 'benchmark'

fib = Fiber.new do
	loop { Fiber.yield }
end

Benchmark.bm do |bm|
	3.times do
		results << bm.report { 10_000_000.times { fib.resume } }
	end
	avg = results.inject(:+) / results.size
        [avg]
end

Raw benchmarking results:

trunk@47827 - 7.59s
patch 4           - 6.59s (87% of trunk)
patch 4+5      - 6.33s (83% of trunk)

[normalperson (Eric Wong)]

Cool, I can confirm the performance results on one of my systems.
I do not know the fiber code well, but it seems correct.

I think fiber_mark_self (and any non-static functions) needs to be
prefixed with "rb_" even if it is an internal API.

[nome (Knut Franke)]

• File 0005-Optimize-fiber_switch-callees.patch 0005-Optimize-fiber_switch-callees.patch added

Attached a revised version of patch 5 with fiber_mark_self replaced by rb_fiber_mark_self. Thanks for the hint.

[normalperson (Eric Wong)]

Thanks. I'll wait a few days for others to look and try it out
before committing.

• Status changed from Open to Closed
• % Done changed from 0 to 100

Applied in changeset r47959.

---

cont.c: Remove unused prev_fiber/next_fiber fields

They were added in r19890 (8a0d53a), with the explanation that it's a
double linked list of fibers in the same thread, but without any code
using them.

• cont.c (rb_fiber_t): remove prev_fiber/next_fiber
  (fiber_link_join, fiber_link_remove): remove functions
  (fiber_free, fiber_init, root_fiber_alloc):
  remove references to removed fields and functions
  [ruby-core:65518] [Feature #10341]

Author: Knut Franke [e-mail]

[nobu (Nobuyoshi Nakada)]

• Status changed from Closed to Open

I've missed this ticket, and r47964 caused compile error.

[nobu (Nobuyoshi Nakada)]

• Status changed from Open to Closed

Applied in changeset r47969.

---

cont.c: fix compile error

• cont.c (rb_fiber_t): fix compile error caused by move to
  vm_core.h at r47964. [Feature #10341]

[ko1 (Koichi Sasada)]

• Status changed from Closed to Open

on mswin32, the following simple script doesn't work.

p Fiber.new{
  100
}.resume

#=>
C:/ko1/src/ruby/trunk/test.rb:1:in `p': method `inspect' called on
hidden T_OBJECT object (0x1ea6730 flags=0x1) (NotImplementedError)
        from C:/ko1/src/ruby/trunk/test.rb:1:in `<main>'
NMAKE : fatal error U1077: '.\miniruby.exe' : リターン コード '0x1'
Stop.

[ko1 (Koichi Sasada)]

disable FIBER_USE_NATIVE works fine on mswin32.

[normalperson (Eric Wong)]

Maybe this is a simple fix for win32, but I cannot test:

diff --git a/cont.c b/cont.c
index 739ec80..08acf40 100644
--- a/cont.c
+++ b/cont.c
@@ -1366,10 +1366,10 @@ fiber_store(rb_fiber_t *next_fib, rb_thread_t th)
terminated_machine_stack.ptr = NULL;
terminated_machine_stack.size = 0;
}
+#endif / not _WIN32 /
fib = th->fiber;
if (fib->cont.argc == -1) rb_exc_raise(fib->cont.value);
return fib->cont.value;
-#endif / not _WIN32 */

#else /* FIBER_USE_NATIVE */
cont_save_machine_stack(th, &fib->cont);

[ko1 (Koichi Sasada)]

Eric Wong wrote:

Maybe this is a simple fix for win32, but I cannot test:

Thank you. Now, it is working. I'm running test-all now.

[ko1 (Koichi Sasada)]

It works fine!

Thank you.

[nome (Knut Franke)]

D'oh. Thanks for fixing the win32/non-native issues.

And great to see this included. :-)

• Status changed from Open to Closed

Applied in changeset trunk|r58606.

---

benchmark/bm_vm2_fiber_switch.rb: check for fiber performance

There are currently no benchmarks for Fiber performance, I
should've committed this years ago when [Feature #10341] was
implemented.