https://bugs.ruby-lang.org/https://bugs.ruby-lang.org/favicon.ico?17113305112022-03-25T20:39:14ZRuby Issue Tracking SystemRuby master - Bug #18663: Autoload doesn't work with fiber context switch.https://bugs.ruby-lang.org/issues/18663?journal_id=970352022-03-25T20:39:14Zioquatix (Samuel Williams)samuel@oriontransfer.net
<ul></ul><p>Pure fiber repro.</p>
<pre><code class="ruby syntaxhl" data-language="ruby"><span class="c1">#!/usr/bin/env ruby</span>
<span class="nb">require</span> <span class="s1">'tempfile'</span>
<span class="no">Tempfile</span><span class="p">.</span><span class="nf">create</span><span class="p">([</span><span class="s1">'foo'</span><span class="p">,</span> <span class="s1">'.rb'</span><span class="p">])</span> <span class="k">do</span> <span class="o">|</span><span class="n">file</span><span class="o">|</span>
<span class="n">file</span><span class="p">.</span><span class="nf">write</span><span class="p">(</span><span class="o"><<~</span><span class="no">RUBY</span><span class="p">)</span><span class="sh">
Fiber.yield
class C
end
</span><span class="no"> RUBY</span>
<span class="n">file</span><span class="p">.</span><span class="nf">close</span>
<span class="nb">autoload</span> <span class="ss">:C</span><span class="p">,</span> <span class="n">file</span><span class="p">.</span><span class="nf">path</span>
<span class="mi">3</span><span class="p">.</span><span class="nf">times</span> <span class="k">do</span> <span class="o">|</span><span class="n">i</span><span class="o">|</span>
<span class="n">fiber</span> <span class="o">=</span> <span class="no">Fiber</span><span class="p">.</span><span class="nf">new</span> <span class="k">do</span>
<span class="vg">$stderr</span><span class="p">.</span><span class="nf">puts</span> <span class="s2">"</span><span class="si">#{</span><span class="n">i</span><span class="si">}</span><span class="s2"> LOADING C"</span>
<span class="vg">$stderr</span><span class="p">.</span><span class="nf">puts</span> <span class="no">C</span>
<span class="k">rescue</span> <span class="no">NameError</span>
<span class="vg">$stderr</span><span class="p">.</span><span class="nf">puts</span> <span class="s2">"NameError: </span><span class="si">#{</span><span class="n">i</span><span class="si">}</span><span class="s2">"</span>
<span class="k">raise</span>
<span class="k">end</span>
<span class="n">fiber</span><span class="p">.</span><span class="nf">resume</span>
<span class="k">end</span>
<span class="k">end</span>
</code></pre> Ruby master - Bug #18663: Autoload doesn't work with fiber context switch.https://bugs.ruby-lang.org/issues/18663?journal_id=970362022-03-25T21:01:38Zioquatix (Samuel Williams)samuel@oriontransfer.net
<ul></ul><pre><code class="c syntaxhl" data-language="c"><span class="cm">/* always on stack, no need to mark */</span>
<span class="k">struct</span> <span class="n">autoload_state</span> <span class="p">{</span>
<span class="k">struct</span> <span class="n">autoload_const</span> <span class="o">*</span><span class="n">ac</span><span class="p">;</span>
<span class="n">VALUE</span> <span class="n">result</span><span class="p">;</span>
<span class="n">VALUE</span> <span class="kr">thread</span><span class="p">;</span>
<span class="k">struct</span> <span class="n">list_head</span> <span class="n">waitq</span><span class="p">;</span>
<span class="p">};</span>
</code></pre>
<p>I'm suspicious of <code>VALUE thread</code>.</p> Ruby master - Bug #18663: Autoload doesn't work with fiber context switch.https://bugs.ruby-lang.org/issues/18663?journal_id=970372022-03-25T21:10:44Zioquatix (Samuel Williams)samuel@oriontransfer.net
<ul></ul><p>Autoload uses a spin lock:</p>
<pre><code class="c syntaxhl" data-language="c"><span class="k">static</span> <span class="n">VALUE</span>
<span class="nf">autoload_sleep</span><span class="p">(</span><span class="n">VALUE</span> <span class="n">arg</span><span class="p">)</span>
<span class="p">{</span>
<span class="k">struct</span> <span class="n">autoload_state</span> <span class="o">*</span><span class="n">state</span> <span class="o">=</span> <span class="p">(</span><span class="k">struct</span> <span class="n">autoload_state</span> <span class="o">*</span><span class="p">)</span><span class="n">arg</span><span class="p">;</span>
<span class="cm">/*
* autoload_reset in other thread will resume us and remove us
* from the waitq list
*/</span>
<span class="k">do</span> <span class="p">{</span>
<span class="n">rb_thread_sleep_deadly</span><span class="p">();</span>
<span class="p">}</span> <span class="k">while</span> <span class="p">(</span><span class="n">state</span><span class="o">-></span><span class="kr">thread</span> <span class="o">!=</span> <span class="n">Qfalse</span><span class="p">);</span>
<span class="k">return</span> <span class="n">Qfalse</span><span class="p">;</span>
<span class="p">}</span>
</code></pre>
<p>It basically spins on <code>state->thread</code>.</p>
<p>I hesitate to call it poorly implemented, but it doesn’t look great. I don’t fully understand the implementation yet.</p> Ruby master - Bug #18663: Autoload doesn't work with fiber context switch.https://bugs.ruby-lang.org/issues/18663?journal_id=970382022-03-25T21:12:25Zioquatix (Samuel Williams)samuel@oriontransfer.net
<ul></ul><p><a class="user active user-mention" href="https://bugs.ruby-lang.org/users/1241">@fxn (Xavier Noria)</a> If I understand correctly, autoload is mostly a feature of a development environment, so if we made this a little bit slower in order to improve accuracy (i.e. using a proper mutex), it wouldn't be huge loss right? Ruby's mutex isn't that slow but I realise if we took this route, it would introduce some overhead.</p> Ruby master - Bug #18663: Autoload doesn't work with fiber context switch.https://bugs.ruby-lang.org/issues/18663?journal_id=970392022-03-25T22:29:39Zfxn (Xavier Noria)fxn@hashref.com
<ul></ul><p><a class="user active user-mention" href="https://bugs.ruby-lang.org/users/3344">@ioquatix (Samuel Williams)</a> hmmm, let me explain.</p>
<p>The feature in development for a web application is reloading. Ordinary gems using Zeitwerk may autoload in any project they are used, and when you eager load Rails in production you may need to autoload top-level constants like superclasses or mixins while eager loading, for example.</p>
<p>However, as the say goes, I can be fast if I can be wrong :). I'd go with that mutex, probably not a big deal, and fibers are key for the future of Ruby, so better be compatible in my view.</p> Ruby master - Bug #18663: Autoload doesn't work with fiber context switch.https://bugs.ruby-lang.org/issues/18663?journal_id=970782022-03-29T22:04:53Zioquatix (Samuel Williams)samuel@oriontransfer.net
<ul></ul><p>I've tracked down the root of this bug, being that it's not yielding to the fiber scheduler and implements it's own condition variable like semantics. I'll propose a fix.</p> Ruby master - Bug #18663: Autoload doesn't work with fiber context switch.https://bugs.ruby-lang.org/issues/18663?journal_id=970902022-03-30T12:22:09ZEregon (Benoit Daloze)
<ul><li><strong>Has duplicate</strong> <i><a class="issue tracker-5 status-5 priority-4 priority-default closed" href="/issues/18662">Misc #18662</a>: Fiber scheduling and Module#autoload</i> added</li></ul> Ruby master - Bug #18663: Autoload doesn't work with fiber context switch.https://bugs.ruby-lang.org/issues/18663?journal_id=975212022-05-07T05:41:36Zioquatix (Samuel Williams)samuel@oriontransfer.net
<ul></ul><p><a href="https://github.com/ruby/ruby/pull/5788" class="external">https://github.com/ruby/ruby/pull/5788</a> fixes this issue.</p>
<p>I've confirmed that my PR fixes the given examples here.</p>
<p>There is a tiny bit of extra overhead; using a mutex has an object allocation, mutex lock and unlock, etc.</p>
<p>A light weight concurrency construct <code>rb_condition</code> or <code>rb_fiber_condition</code> that behaves like a "pessimistic mutex", i.e. avoids the allocation until it's actually needed might be a better solution but let's get it correct first, and we can optimise it later.</p>
<p>Just a few notes:</p>
<pre><code class="c syntaxhl" data-language="c"><span class="k">struct</span> <span class="n">rb_condition</span> <span class="p">{</span>
<span class="p">...</span> <span class="n">waitq</span><span class="p">;</span>
<span class="p">}</span>
<span class="n">VALUE</span> <span class="nf">rb_condition_wait</span><span class="p">(</span><span class="k">struct</span> <span class="n">rb_condition</span> <span class="o">*</span><span class="n">condition</span><span class="p">,</span> <span class="n">VALUE</span> <span class="n">timeout</span><span class="p">);</span>
<span class="n">rb_condition_signal</span><span class="p">(</span><span class="k">struct</span> <span class="n">rb_condition</span> <span class="o">*</span><span class="n">condition</span><span class="p">,</span> <span class="n">VALUE</span> <span class="n">result</span><span class="p">);</span> <span class="c1">// wake up all waiters</span>
</code></pre> Ruby master - Bug #18663: Autoload doesn't work with fiber context switch.https://bugs.ruby-lang.org/issues/18663?journal_id=975222022-05-07T08:18:50Zfxn (Xavier Noria)fxn@hashref.com
<ul></ul><p>❤️</p> Ruby master - Bug #18663: Autoload doesn't work with fiber context switch.https://bugs.ruby-lang.org/issues/18663?journal_id=975232022-05-07T10:14:38Znagachika (Tomoyuki Chikanaga)nagachika00@gmail.com
<ul><li><strong>Backport</strong> changed from <i>2.6: UNKNOWN, 2.7: UNKNOWN, 3.0: UNKNOWN, 3.1: UNKNOWN</i> to <i>2.7: DONTNEED, 3.0: REQUIRED, 3.1: REQUIRED</i></li></ul> Ruby master - Bug #18663: Autoload doesn't work with fiber context switch.https://bugs.ruby-lang.org/issues/18663?journal_id=975252022-05-07T22:24:15Zioquatix (Samuel Williams)samuel@oriontransfer.net
<ul><li><strong>Status</strong> changed from <i>Open</i> to <i>Closed</i></li></ul><p>I've merged the fix.</p>