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

Optimized __method__ and __callee__ #8172

Merged
merged 7 commits into from
Mar 29, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
43 changes: 43 additions & 0 deletions bench/bench_frame_name_methods.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
require 'benchmark/ips'

def foo_method
__method__
end

def foo_callee
__callee__
end

alias bar_method foo_method
alias bar_callee foo_callee

Benchmark.ips do |bm|
bm.report("__method__ same") do |i|
while i > 0
i-=1
foo_method;foo_method;foo_method;foo_method;foo_method;foo_method;foo_method;foo_method;foo_method;foo_method
end
end

bm.report("__method__ different") do |i|
while i > 0
i-=1
bar_method;bar_method;bar_method;bar_method;bar_method;bar_method;bar_method;bar_method;bar_method;bar_method
end
end


bm.report("__callee__ same") do |i|
while i > 0
i-=1
foo_callee;foo_callee;foo_callee;foo_callee;foo_callee;foo_callee;foo_callee;foo_callee;foo_callee;foo_callee
end
end

bm.report("__callee__ different") do |i|
while i > 0
i-=1
bar_callee;bar_callee;bar_callee;bar_callee;bar_callee;bar_callee;bar_callee;bar_callee;bar_callee;bar_callee
end
end
end
52 changes: 52 additions & 0 deletions core/src/main/java/org/jruby/RubySymbol.java
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,9 @@
import org.jruby.util.TypeConverter;

import java.lang.ref.WeakReference;
import java.util.AbstractMap;
import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.locks.ReentrantLock;

import static org.jruby.util.RubyStringBuilder.str;
Expand Down Expand Up @@ -346,6 +349,14 @@ public static RubySymbol newHardSymbol(Ruby runtime, String name) {
return runtime.getSymbolTable().getSymbol(name, true);
}

public static RubySymbol newMethodSymbolFromCompound(Ruby runtime, String compoundName) {
return runtime.getSymbolTable().getMethodSymbolFromCompound(compoundName);
}

public static RubySymbol newCalleeSymbolFromCompound(Ruby runtime, String compoundName) {
return runtime.getSymbolTable().getCalleeSymbolFromCompound(compoundName);
}

/**
* Return the symbol in the symbol table if it exists, null otherwise.
* This method will not create the symbol if it does not exist.
Expand Down Expand Up @@ -979,6 +990,7 @@ public static final class SymbolTable {

private final ReentrantLock tableLock = new ReentrantLock();
private volatile SymbolEntry[] symbolTable;
private Map<String, CompoundSymbol> compoundSymbolTable = new ConcurrentHashMap<>();
private int size;
private int threshold;
private final float loadFactor;
Expand Down Expand Up @@ -1093,6 +1105,46 @@ public RubySymbol getSymbol(ByteList bytes, ObjBooleanConsumer<RubySymbol> handl
return symbol;
}

/**
* Get the method name symbol from a compound name.
*
* @see #getCompoundSymbol(String)
* @param compoundName the compound name
* @return the method component of the compound name, as a symbol
*/
public RubySymbol getMethodSymbolFromCompound(String compoundName) {
return getCompoundSymbol(compoundName).method;
}


/**
* Get the callee name symbol from a compound name.
*
* @see #getCompoundSymbol(String)
* @param compoundName the compound name
* @return the callee component of the compound name, as a symbol
*/
public RubySymbol getCalleeSymbolFromCompound(String compoundName) {
return getCompoundSymbol(compoundName).callee;
}

/**
* Get a pair of symbols associated with the given compound method name, used by aliases to pass both the callee
* name and the original method name on the stack. This avoids re-parsing the compoundName and constructing new
* strings every time __method__ or __callee__ are used in an aliased call.
*
* @param compoundName the compound name used for a combination of alias and method
* @return a Map.Entry representing the __method__ and __callee__ symbols for that compound name as key and value
*/
private CompoundSymbol getCompoundSymbol(String compoundName) {
return compoundSymbolTable.computeIfAbsent(compoundName, (cname) ->
new CompoundSymbol(
getSymbol(Helpers.getSuperNameFromCompositeName(cname), true),
getSymbol(Helpers.getCalleeNameFromCompositeName(cname), true)));
}

private record CompoundSymbol(RubySymbol method, RubySymbol callee){}

private RubySymbol findSymbol(ByteList bytes, int hash, boolean hard) {
RubySymbol symbol = null;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ private IRubyObject INTERPRET_CLASS(InterpreterContext ic, ThreadContext context
private IRubyObject interpretWithBacktrace(InterpreterContext ic, ThreadContext context, IRubyObject self, String name, Block block) {
try {
ThreadContext.pushBacktrace(context, name, ic.getFileName(), ic.getLine());
return ic.getEngine().interpret(context, null, self, ic, getImplementationClass().getMethodLocation(), name, block);
return ic.getEngine().interpret(context, null, self, ic, getImplementationClass().getMethodLocation(), null, block);
} finally {
ThreadContext.popBacktrace(context);
}
Expand Down
1 change: 1 addition & 0 deletions core/src/main/java/org/jruby/ir/IRVisitor.java
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ private void error(Object object) {
public void EQQInstr(EQQInstr eqqinstr) { error(eqqinstr); }
public void ExceptionRegionEndMarkerInstr(ExceptionRegionEndMarkerInstr exceptionregionendmarkerinstr) { error(exceptionregionendmarkerinstr); }
public void ExceptionRegionStartMarkerInstr(ExceptionRegionStartMarkerInstr exceptionregionstartmarkerinstr) { error(exceptionregionstartmarkerinstr); }
public void FrameNameCallInstr(FrameNameCallInstr framenamecallinstr) { error(framenamecallinstr); }
public void GetClassVarContainerModuleInstr(GetClassVarContainerModuleInstr getclassvarcontainermoduleinstr) { error(getclassvarcontainermoduleinstr); }
public void GetClassVariableInstr(GetClassVariableInstr getclassvariableinstr) { error(getclassvariableinstr); }
public void GetFieldInstr(GetFieldInstr getfieldinstr) { error(getfieldinstr); }
Expand Down
1 change: 1 addition & 0 deletions core/src/main/java/org/jruby/ir/Operation.java
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ public enum Operation {
CALL_0O(OpFlags.f_has_side_effect | OpFlags.f_is_call | OpFlags.f_can_raise_exception),
NORESULT_CALL_1O(OpFlags.f_has_side_effect | OpFlags.f_is_call | OpFlags.f_can_raise_exception),
BLOCK_GIVEN_CALL(OpFlags.f_has_side_effect | OpFlags.f_is_call | OpFlags.f_can_raise_exception),
FRAME_NAME_CALL(OpFlags.f_has_side_effect | OpFlags.f_is_call | OpFlags.f_can_raise_exception),

/** Ruby operators: should all these be calls? Implementing instrs don't inherit from CallBase.java */
EQQ(OpFlags.f_has_side_effect | OpFlags.f_can_raise_exception), // a === call used in when
Expand Down
13 changes: 13 additions & 0 deletions core/src/main/java/org/jruby/ir/builder/IRBuilderAST.java
Original file line number Diff line number Diff line change
Expand Up @@ -2486,6 +2486,19 @@ public Operand buildVAlias(VAliasNode valiasNode) {
}

public Operand buildVCall(Variable result, VCallNode node) {
if (result == null) result = temp();

RubySymbol name = methodName = node.getName();

// special case methods with frame handling
String callName = name.idString();
switch (callName) {
case "__method__":
case "__callee__":
addInstr(new FrameNameCallInstr(result, callName));
return result;
}

return _call(result, VARIABLE, buildSelf(), node.getName());
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ private IRubyObject INTERPRET_CLASS(ThreadContext context, RubyModule clazz) {

try {
ThreadContext.pushBacktrace(context, id, ic.getFileName(), ic.getLine());
return ic.getEngine().interpret(context, null, clazz, ic, clazz.getMethodLocation(), id, Block.NULL_BLOCK);
return ic.getEngine().interpret(context, null, clazz, ic, clazz.getMethodLocation(), null, Block.NULL_BLOCK);
} finally {
body.cleanupAfterExecution();
if (!hasExplicitCallProtocol) post(ic, context);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ private IRubyObject INTERPRET_MODULE(ThreadContext context, RubyModule clazz) {

try {
ThreadContext.pushBacktrace(context, id, ic.getFileName(), ic.getLine());
return ic.getEngine().interpret(context, null, clazz, ic, clazz.getMethodLocation(), id, Block.NULL_BLOCK);
return ic.getEngine().interpret(context, null, clazz, ic, clazz.getMethodLocation(), null, Block.NULL_BLOCK);
} finally {
body.cleanupAfterExecution();
if (!hasExplicitCallProtocol) post(ic, context);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
package org.jruby.ir.instructions;

import org.jruby.RubySymbol;
import org.jruby.ir.IRVisitor;
import org.jruby.ir.Operation;
import org.jruby.ir.operands.Variable;
import org.jruby.ir.persistence.IRReaderDecoder;
import org.jruby.ir.persistence.IRWriterEncoder;
import org.jruby.ir.transformations.inlining.CloneInfo;
import org.jruby.runtime.ThreadContext;
import org.jruby.runtime.builtin.IRubyObject;
import org.jruby.runtime.callsite.CacheEntry;
import org.jruby.runtime.callsite.VariableCachingCallSite;

import java.util.Objects;

/**
* A call to __method__ or __callee__ which can be optimized to use the frame method name directly.
*/
public class FrameNameCallInstr extends NoOperandResultBaseInstr implements FixedArityInstr {
private final VariableCachingCallSite frameNameSite;
private final boolean callee;

public FrameNameCallInstr(Variable result, String methodName) {
super(Operation.FRAME_NAME_CALL, Objects.requireNonNull(result, "FrameNameCallInstr result is null"));

this.frameNameSite =
new VariableCachingCallSite(Objects.requireNonNull(methodName, "FrameNameCallInstr methodName is null"));

this.callee = methodName.equals("__callee__");
}

public String getMethodName() {
return frameNameSite.getMethodName();
}

@Override
public Instr clone(CloneInfo ii) {
return new FrameNameCallInstr(ii.getRenamedVariable(result), frameNameSite.getMethodName());
}

public static FrameNameCallInstr decode(IRReaderDecoder d) {
return new FrameNameCallInstr(d.decodeVariable(), d.decodeString());
}

@Override
public void encode(IRWriterEncoder e) {
super.encode(e);
e.encode(frameNameSite.getMethodName());
}

public IRubyObject getFrameName(ThreadContext context, IRubyObject self, String compositeName) {
CacheEntry frameNameEntry = frameNameSite.retrieveCache(self);

if (!frameNameEntry.method.getRealMethod().isBuiltin()) {
return frameNameSite.call(context, self, self);
}

if (compositeName == null) return context.nil;

return callee ?
RubySymbol.newCalleeSymbolFromCompound(context.runtime, compositeName):
RubySymbol.newMethodSymbolFromCompound(context.runtime, compositeName);
}

@Override
public void visit(IRVisitor visitor) {
visitor.FrameNameCallInstr(this);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ public ExitableReturn interpret(ThreadContext context, Block block, IRubyObject
break;
case CALL_OP:
if (profile) Profiler.updateCallSite(instr, interpreterContext.getScope(), scopeVersion);
processCall(context, instr, operation, currDynScope, currScope, temp, self);
processCall(context, instr, operation, currDynScope, currScope, temp, self, name, block);
break;
case RET_OP:
processReturnOp(context, block, instr, operation, currDynScope, temp, self, currScope);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
import org.jruby.ir.instructions.CheckArityInstr;
import org.jruby.ir.instructions.CheckForLJEInstr;
import org.jruby.ir.instructions.CopyInstr;
import org.jruby.ir.instructions.FrameNameCallInstr;
import org.jruby.ir.instructions.Instr;
import org.jruby.ir.instructions.JumpInstr;
import org.jruby.ir.instructions.LineNumberInstr;
Expand Down Expand Up @@ -150,7 +151,7 @@ public IRubyObject interpret(ThreadContext context, Block block, IRubyObject sel
break;
case CALL_OP:
if (profile) Profiler.updateCallSite(instr, interpreterContext.getScope(), scopeVersion);
processCall(context, instr, operation, currDynScope, currScope, temp, self);
processCall(context, instr, operation, currDynScope, currScope, temp, self, name, block);
break;
case RET_OP:
return processReturnOp(context, block, instr, operation, currDynScope, temp, self, currScope);
Expand Down Expand Up @@ -286,7 +287,7 @@ protected static void receiveArg(ThreadContext context, Instr i, Operation opera
}
}

protected static void processCall(ThreadContext context, Instr instr, Operation operation, DynamicScope currDynScope, StaticScope currScope, Object[] temp, IRubyObject self) {
protected static void processCall(ThreadContext context, Instr instr, Operation operation, DynamicScope currDynScope, StaticScope currScope, Object[] temp, IRubyObject self, String frameName, Block selfBlock) {
Object result;

switch(operation) {
Expand Down Expand Up @@ -358,6 +359,11 @@ protected static void processCall(ThreadContext context, Instr instr, Operation
case NORESULT_CALL:
instr.interpret(context, currScope, currDynScope, self, temp);
break;
case FRAME_NAME_CALL:
setResult(temp, currDynScope, instr,
((FrameNameCallInstr) instr).getFrameName(
context, self, selfBlock == null ? frameName : IRRuntimeHelpers.getFrameNameFromBlock(selfBlock)));
break;
case CALL:
default:
result = instr.interpret(context, currScope, currDynScope, self, temp);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ public IRubyObject interpret(ThreadContext context, Block block, IRubyObject sel
break;
case CALL_OP:
if (profile) Profiler.updateCallSite(instr, interpreterContext.getScope(), scopeVersion);
processCall(context, instr, operation, currDynScope, currScope, temp, self);
processCall(context, instr, operation, currDynScope, currScope, temp, self, name, block);
break;
case RET_OP:
return processReturnOp(context, block, instr, operation, currDynScope, temp, self, currScope);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -295,6 +295,7 @@ public Instr decodeInstr() {
case EQQ: return EQQInstr.decode(this);
case EXC_REGION_END: return new ExceptionRegionEndMarkerInstr();
case EXC_REGION_START: return ExceptionRegionStartMarkerInstr.decode(this);
case FRAME_NAME_CALL: return FrameNameCallInstr.decode(this);
case GET_CVAR: return GetClassVariableInstr.decode(this);
case GET_ENCODING: return GetEncodingInstr.decode(this);
case GET_ERROR_INFO: return GetErrorInfoInstr.decode(this);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1964,7 +1964,7 @@ public static void defCompiledInstanceMethod(ThreadContext context, MethodHandle
public static IRubyObject invokeModuleBody(ThreadContext context, DynamicMethod method) {
RubyModule implClass = method.getImplementationClass();

return method.call(context, implClass, implClass, "", Block.NULL_BLOCK);
return method.call(context, implClass, implClass, null, Block.NULL_BLOCK);
}

@JIT
Expand Down Expand Up @@ -2662,4 +2662,10 @@ private static IRRuntimeHelpersSites sites(ThreadContext context) {
public static int arrayLength(RubyArray array) {
return array.getLength();
}

@Interp @JIT
public static String getFrameNameFromBlock(Block block) {
// FIXME: binding.getMethod does not appear to be the right name in defined_method bodies... WHY?
return block.getBinding().getFrame().getName();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -318,8 +318,7 @@ public void loadFrameName() {
if (superNameOffset == -1) {
// load from self block
loadSelfBlock();
adapter.invokevirtual(p(Block.class), "getBinding", sig(Binding.class));
adapter.invokevirtual(p(Binding.class), "getMethod", sig(String.class));
adapter.invokestatic(p(IRRuntimeHelpers.class), "getFrameNameFromBlock", sig(String.class, Block.class));
} else {
adapter.aload(superNameOffset);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -133,4 +133,9 @@ public interface InvocationCompiler {
* Invoke block_given? or iterator? with awareness of any built-in methods.
*/
void invokeBlockGiven(String methodName, String file);

/**
* Invoke __method__ or __callee__ with awareness of any built-in methods.
*/
void invokeFrameName(String methodName, String file);
}
6 changes: 6 additions & 0 deletions core/src/main/java/org/jruby/ir/targets/JVMVisitor.java
Original file line number Diff line number Diff line change
Expand Up @@ -1650,6 +1650,12 @@ public void ExceptionRegionStartMarkerInstr(ExceptionRegionStartMarkerInstr exce
throw new NotCompilableException("Marker instructions shouldn't reach compiler: " + exceptionregionstartmarkerinstr);
}

@Override
public void FrameNameCallInstr(FrameNameCallInstr framenamecallinstr) {
jvmMethod().getInvocationCompiler().invokeFrameName(framenamecallinstr.getMethodName(), file);
handleCallResult(jvmMethod(), framenamecallinstr.getResult());
}

@Override
public void GetClassVarContainerModuleInstr(GetClassVarContainerModuleInstr getclassvarcontainermoduleinstr) {
jvmMethod().loadContext();
Expand Down