Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RubyEnumerator.SizeFn does not receive enough context #4688

Closed
headius opened this issue Jun 26, 2017 · 5 comments
Closed

RubyEnumerator.SizeFn does not receive enough context #4688

headius opened this issue Jun 26, 2017 · 5 comments

Comments

@headius
Copy link
Member

headius commented Jun 26, 2017

In order to implement enumerators with sizes, we have a SizeFn interface we implement for various types of collection and enumeration. However this interface currently only receives IRubyObject[] which means the runtime, context, self, and other contextual values must be captured (usually as an anonymous inner class) or acquired (e.g. runtime.getCurrentContext()).

This interface should be expanded to accept at least context and self, and all uses should be audited to see if we can make the impls static instances rather than creating them anew each time.

@anukin
Copy link
Contributor

anukin commented Jul 12, 2017

@headius I have raised a PR here #4714. Can you take a look ?

@headius
Copy link
Member Author

headius commented Jul 13, 2020

@anukin I have reworked your original PR and will finish the remaining requirements of this issue as part of #6321. Thank you for your help, even though it got left behind.

headius added a commit to headius/jruby that referenced this issue Jul 13, 2020
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.
@anukin
Copy link
Contributor

anukin commented Jul 19, 2020

Hey thanks :)

headius added a commit to headius/jruby that referenced this issue Sep 23, 2020
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.
@edipofederle
Copy link
Contributor

edipofederle commented Jun 15, 2021

@headius could we close this issue?

@headius
Copy link
Member Author

headius commented Jun 15, 2021

@edipofederle Yeah I guess this should have gone along with #6321. Thanks for the audit!

@headius headius added this to the JRuby 9.3.0.0 milestone Jun 15, 2021
@headius headius closed this as completed Jun 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants