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.

Sign up for GitHub

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

Jump to bottom

[Truffle] Support ObjectSpace.each_object for classes without deoptimisation #3501

Closed
chrisseaton opened this issue Nov 27, 2015 · 6 comments
Closed

[Truffle] Support ObjectSpace.each_object for classes without deoptimisation #3501

chrisseaton opened this issue Nov 27, 2015 · 6 comments
Labels
beginner
Milestone
Non-Release

Comments

@chrisseaton
Copy link
Contributor

Truffle supports ObjectSpace.each_object but it is currently implemented using stop-and-deoptimise-the-world, which means it's expensive to use.

That's ok, as ObjectSpace.each_object is pretty rare, and is probably (I'm guessing - no empirical data) that it's generally used in things like testing frameworks. There is one exception, which is when it's used with classes.

For example: https://github.com/rails/rails/blob/master/activesupport/lib/active_support/core_ext/class/subclasses.rb

JRuby support this specific case by maintaining a weak set of potentially live classes, which they add to whenever a class is created. We should do the same.

Important: check how this interacts with singleton classes, which we may allocate in inner loops. It should just work with singleton classes on classes and modules, I believe.
@chrisseaton chrisseaton added this to the truffle-dev milestone Nov 27, 2015
@robacarp
Copy link

For another reference, Sidekiq is using ObjectSpace though it wouldn't hurt it that much to hit a hard stop as it hits only at boot:

@chrisseaton
Copy link
Contributor Author

@nirvdrum do you know how JRuby is able to run Sidekiq when it's using each_object with File?

@kares
Copy link
Member

kares commented Feb 22, 2016

@chrisseaton each_object works for class/module argument by default as those objects are already tracked by the runtime:

jruby/core/src/main/java/org/jruby/RubyObjectSpace.java

Lines 124 to 149 in f9d03ff

if (args.length == 0) {
tmpClass = recv.getRuntime().getObject();
} else {
if (!(args[0] instanceof RubyModule)) throw recv.getRuntime().newTypeError("class or module required");
tmpClass = (RubyModule) args[0];
}
final RubyModule rubyClass = tmpClass;
Ruby runtime = recv.getRuntime();
final int[] count = {0};
if (rubyClass == runtime.getClassClass() ||
rubyClass == runtime.getModule()) {
final Collection<IRubyObject> modules = new ArrayList<IRubyObject>();
runtime.eachModule(new Function1<Object, IRubyObject>() {
public Object apply(IRubyObject arg1) {
if (rubyClass.isInstance(arg1)) {
if (arg1 instanceof IncludedModule ||
(arg1 instanceof RubyClass && ((RubyClass)arg1).isSingleton())) {
// do nothing for included wrappers or singleton classes
} else {
count[0]++;
modules.add(arg1); // store the module to avoid concurrent modification exceptions
}
}
return null;
}
});

@chrisseaton
Copy link
Contributor Author

@kares yes but this is calling it with File.

irb(main):002:0> ObjectSpace.each_object(File).count
RuntimeError: ObjectSpace is disabled; each_object will only work with Class, pass -X+O to enable
    from org/jruby/RubyObjectSpace.java:185:in `each_object'
    from org/jruby/RubyEnumerator.java:293:in `each'
    from org/jruby/RubyEnumerable.java:151:in `count'

@kares
Copy link
Member

kares commented Feb 22, 2016

oh right, that's interesting. believe its not run with --daemon or handling USR2 under JRuby than.

@chrisseaton
Copy link
Contributor Author

Closing because we've moved out of this repository.

@enebo enebo added this to the Non-Release milestone Dec 7, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
beginner
Projects
None yet
Milestone
Non-Release
Development

No branches or pull requests
4 participants
@enebo @kares @robacarp @chrisseaton