Skip to content

Add self to SizeFn#6321

Merged
headius merged 7 commits intojruby:masterfrom
headius:better_enumerator_sizefn
Sep 24, 2020
Merged

Add self to SizeFn#6321
headius merged 7 commits intojruby:masterfrom
headius:better_enumerator_sizefn

Conversation

@headius
Copy link
Member

@headius headius commented Jul 13, 2020

This is a cleaned-up merge of @anukin's changes in #4714.

The addition of context happened separately as part of #5958, and as a result this PR only adds the "self" receiver to the SizeFn interface.

This PR may be fine to include as-is, but none of the changes involved actually use the self object for anything. This PR as submitted only updates #4714 to merge properly.

Additional work to follow, which would then actually complete work described in #4688, would be to make all SizeFn objects static instances, using the passed in "self" to do the stateful calls they make now against the captured self. This would avoid the repeated allocation of the SizeFn instances for each enumerator produced by a given enumerable object.

@headius
Copy link
Member Author

headius commented Jul 13, 2020

@enebo @kares the most recent commit is the most invasive, mostly because it adds a generic <T extends IRubyObject> parameter to RubyEnumerator.SizeFn.

Copy link
Member

@kares kares left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 believe this also provides better readability.
despite the bigger change-set it reads nicely ...

This eliminates the class for all remaining SizeFn inner class
implementations and replaces all stateful inner class and lambda
implementations of SizeFn with method references. This completes
all requirements for jruby#4688.

Method references are zero-alloc after the initial access, and
are initialized lazily as opposed to anonymous inner classes
(allocated every time) or static impl references (initialized at
class init time). In addition, through the magic of lambdas this
eliminates any remaining inner class instances, shrinking our
class load slightly.

The only controversial change here is the generification of SizeFn
to allow the passed-in self to be of the same type as the object
that serves as the enunmeration base. This might introduce some
binary incompatibility with external libraries, but I do not know
of any such libraries that implement their own SizeFn.
@headius
Copy link
Member Author

headius commented Jul 14, 2020

There's some quirks I have to work through here around Enumerator, since it seems the size function there needs to always operate against the enumerator rather than the object it wraps. Will return to this soon, but if someone else wants to try it you can see quick failures running test/jruby/test_enumerator.rb or rake test:jruby.

@headius headius force-pushed the better_enumerator_sizefn branch from 97ef716 to e6e15e0 Compare September 23, 2020 23:35
* Always pass object through to sizeFn
* Always use Enumerable sizeFn for new Enumerator
@headius headius force-pushed the better_enumerator_sizefn branch from c231eb4 to 0aa6d46 Compare September 24, 2020 00:27
@headius headius merged commit e7ef8f4 into jruby:master Sep 24, 2020
@headius headius deleted the better_enumerator_sizefn branch September 24, 2020 02:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants