Skip to content

Commit

Permalink
Showing 1 changed file with 13 additions and 2 deletions.
Original file line number Diff line number Diff line change
@@ -10,22 +10,33 @@
package org.jruby.truffle.language.arguments;

import com.oracle.truffle.api.frame.VirtualFrame;
import com.oracle.truffle.api.interop.TruffleObject;
import com.oracle.truffle.api.profiles.ConditionProfile;
import com.oracle.truffle.api.profiles.PrimitiveValueProfile;
import com.oracle.truffle.api.profiles.ValueProfile;
import org.jruby.truffle.language.RubyNode;

public class ProfileArgumentNode extends RubyNode {

@Child private RubyNode readArgumentNode;

private final ValueProfile valueProfile = ValueProfile.createClassProfile();
private final ConditionProfile objectProfile = ConditionProfile.createBinaryProfile();
private final ValueProfile primitiveProfile = PrimitiveValueProfile.createEqualityProfile();
private final ValueProfile classProfile = ValueProfile.createClassProfile();

public ProfileArgumentNode(RubyNode readArgumentNode) {
this.readArgumentNode = readArgumentNode;
}

@Override
public Object execute(VirtualFrame frame) {
return valueProfile.profile(readArgumentNode.execute(frame));
final Object value = readArgumentNode.execute(frame);

if (objectProfile.profile(value instanceof TruffleObject)) {
return classProfile.profile(value);
} else {
return primitiveProfile.profile(value);
}
}

}

5 comments on commit 07128c4

@chrisseaton
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@thedarkone this solves the problem of profiles hanging on to arbitrary objects. We now profile primitives for their value, and objects only for their Java class.

@thedarkone
Copy link
Contributor

Choose a reason for hiding this comment

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

Wow, you haven't forgotten about that one!

This is a right move, I still think remembering obj identity buys us really little in Ruby (e.g. it doesn't mean the shape of the obj or the class of the ruby obj hasn't changed, etc.)

I'm wondering if the ruby-level class profile is even worth it, since Truffle does its own .getClass() profiling of call target args (and does it in a genius way, by profiling at the call site where the type information is almost always "freely" available in the Graal IR).

@chrisseaton
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I don't understand here why the Truffle class profiling doesn't make the class profile here redundant. But if you remove either of the two profiles above benchmarks will drop and compiler tests will fail. We tried a lot of different ways of profiling, and this did the best on average. I don't really know why!

@thedarkone
Copy link
Contributor

Choose a reason for hiding this comment

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

Well, if I remember correctly, it does get disabled if the call target is called with variable arg lengths (ie it gives up right away and doesn't attempt to profile a max prefix) or if the call target is ever used as a target of an indirect call.

Do you have an exemplary (smallish 😄) bench?

@chrisseaton
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't have one benchmark in particular - part of the problem was it varied for benchmarks and we needed to look at the aggregate.

But look at the PSD compose benchmarks (https://github.com/jruby/all-ruby-benchmarks/tree/master/psd.rb) and the acid benchmark (https://github.com/jruby/all-ruby-benchmarks/blob/master/synthetic/acid.rb). They varied a lot for different kinds of profiles.

Please sign in to comment.