Use pure-Ruby Fiber-based Enumerator#next#5672
Merged
headius merged 9 commits intojruby:masterfrom Apr 12, 2019
Merged
Conversation
This code has been passed around multiple implementations as a pure-Ruby version of Enumerator#next that just uses fibers. We have had many issues with the separate fiber-like impl we used for Enumerator#next, and the logical path forward is to unify that with Fiber via this code. There are a few caveats to this migration: * Three new introduced failures appear to be shared with other impls using this logic (rbx, truffleruby). * This commit breaks the fast-path Array logic for Enumerator#next so it's always going through a thread. However the protocol here would allow us to implement #to_generator on any object, which should make it possible to short-circuit more easily (in Ruby!). * I have not measured performance. However the logic surrounding the Fiber calls should not be particularly heavy. * I have not vetted the cleanup side of this, except to confirm that this implementation seems about as good at jruby#5671 in that it does appear to GC, but for a high-churn benchmark it eventually collapses without explicit GCs. So...no worse? This code is modified from TruffleRuby's copy of Rubinius's code, with Rubinius's license being very liberal and TruffleRuby's being identical to JRuby's.
kares
requested changes
Apr 2, 2019
guizmaii
referenced
this pull request
in Colisweb/AdoptJRuby
Apr 3, 2019
guizmaii
referenced
this pull request
in Colisweb/AdoptJRuby
Apr 3, 2019
- Use my repo instead of the headius one. - build Jruby with `./mvnw` instead of `mvn package -Pbootstrap` - clean
guizmaii
referenced
this pull request
in Colisweb/AdoptJRuby
Apr 3, 2019
guizmaii
referenced
this pull request
in Colisweb/AdoptJRuby
Apr 3, 2019
guizmaii
referenced
this pull request
in Colisweb/AdoptJRuby
Apr 3, 2019
WIP: Build `https://github.com/jruby/jruby/pull/5672` JRuby version
The original implementation was based on Enumerator code from Rubinius. However in Rubinius, the Fiber thread does not root objects any deeper than to the Fiber object itself. In order to make this not leak threads, all state shared between the Fiber and the Enumerator is passed in a separate State object. This also includes minor tweaks, like fixing the to_generator logic (needed for efficient non-fiber generators), and using the Fiber.yield method directly as the body of the iteration, so no intermediate block state is necessary. This last change should reduce the cost of the iteration loop close to what it was in the Java-based Enumerator#next.
* Fix generator result * Fix peek_values to return a new array each time * Call finalize on still-alive fibers when rewinding, to attempt to eagerly kill them.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This code has been passed around multiple implementations as a
pure-Ruby version of Enumerator#next that just uses fibers. We
have had many issues with the separate fiber-like impl we used
for Enumerator#next, and the logical path forward is to unify that
with Fiber via this code.
There are a few caveats to this migration:
impls using this logic (rbx, truffleruby).
so it's always going through a thread. However the protocol here
would allow us to implement #to_generator on any object, which
should make it possible to short-circuit more easily (in Ruby!).
the Fiber calls should not be particularly heavy.
that this implementation seems about as good at Thread leak with the
bizgem #5671in that it does appear to GC, but for a high-churn benchmark it
eventually collapses without explicit GCs. So...no worse?
This code is modified from TruffleRuby's copy of Rubinius's code,
with Rubinius's license being very liberal and TruffleRuby's being
identical to JRuby's.
I am proposing this for 9.2.7 because it seems like mostly a lateral move, although there were some tag changes. There are at least two todos before release: