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

[GR-45621] Optimize calls with ruby2_keywords forwarding by deciding it per call site #3270

Merged
merged 4 commits into from
Sep 21, 2023
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ test/mri/tests/cext-c/**/depend
# Diagnostic output directory
dumps/
graal_dumps/
/native-image-build-rubyvm.json

# Profiling
profiles/
Expand Down
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ Compatibility:

Performance:

* Optimize calls with `ruby2_keywords` forwarding by deciding it per call site instead of per callee thanks to [my fix in CRuby 3.2](https://bugs.ruby-lang.org/issues/18625) (@eregon).

Changes:

Expand Down
2 changes: 1 addition & 1 deletion mx.truffleruby/env_files.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ Here is how the various env files relate to each other:
* `jvm-ee-ntl`: + native toolchain launchers
* `jvm-ee-libgraal`: + libgraal
* `native-ee`: + librubyvm + `Truffle Macro Enterprise` + Native Image G1
* `native-ee-host-inlining`: + `TruffleHostInliningPrintExplored`, - native toolchain launchers
* `native-ee-aux`: + `AuxiliaryEngineCache`, - Native Image G1 (currently incompatible)
* `jvm-gu`: + Graal Updater
* `jvm-js`: + Graal.js
* `jvm-py`: + GraalPython
3 changes: 0 additions & 3 deletions mx.truffleruby/jvm-gu

This file was deleted.

9 changes: 9 additions & 0 deletions mx.truffleruby/native-ee-host-inlining
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
GRAALVM_SKIP_ARCHIVE=true
DYNAMIC_IMPORTS=/tools,/truffleruby-enterprise,/graal-enterprise,/substratevm-enterprise,substratevm-enterprise-gcs
COMPONENTS=TruffleRuby,suite:tools,GraalVM enterprise compiler,Truffle enterprise,SubstrateVM Enterprise,Truffle Macro Enterprise,suite:substratevm-enterprise-gcs
NATIVE_IMAGES=lib:rubyvm
EXTRA_IMAGE_BUILDER_ARGUMENTS=rubyvm:-H:+UnlockExperimentalVMOptions rubyvm:-H:BuildOutputJSONFile=native-image-build-rubyvm.json rubyvm:-H:Log=HostInliningPhase,~CanonicalizerPhase,~GraphBuilderPhase rubyvm:-H:+TruffleHostInliningPrintExplored rubyvm:-H:-UnlockExperimentalVMOptions rubyvm:-Dgraal.LogFile=host-inlining.txt
GENERATE_DEBUGINFO=false
# To also create the standalone
INSTALLABLES=TruffleRuby
BUILD_TARGETS=GRAALVM,GRAALVM_STANDALONES
1 change: 0 additions & 1 deletion spec/tags/core/module/ruby2_keywords_tags.txt

This file was deleted.

1 change: 0 additions & 1 deletion spec/tags/language/keyword_arguments_tags.txt
Original file line number Diff line number Diff line change
@@ -1,2 +1 @@
fails:Keyword arguments raises ArgumentError for missing keyword arguments even if there are extra ones
fails:Keyword arguments delegation does not work with call(*ruby2_keyword_args) with missing ruby2_keywords in between
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ ast: |
methodName = "convert"
notEmptyKeywordsProfile = false
notRuby2KeywordsHashProfile = false
ruby2KeywordsHashProfile = false
children:
arguments = [
IntegerFixnumLiteralNode
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ ast: |
methodName = "convert"
notEmptyKeywordsProfile = false
notRuby2KeywordsHashProfile = false
ruby2KeywordsHashProfile = false
children:
arguments = [
IntegerFixnumLiteralNode
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,6 @@ ast: |
methodName = "bar"
notEmptyKeywordsProfile = false
notRuby2KeywordsHashProfile = false
ruby2KeywordsHashProfile = false
children:
receiver =
SelfNode
Expand Down
1 change: 0 additions & 1 deletion spec/truffle/parsing/fixtures/ensure/in_module.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,6 @@ ast: |
methodName = "bar"
notEmptyKeywordsProfile = false
notRuby2KeywordsHashProfile = false
ruby2KeywordsHashProfile = false
children:
receiver =
SelfNode
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ ast: |
methodName = "foo"
notEmptyKeywordsProfile = false
notRuby2KeywordsHashProfile = false
ruby2KeywordsHashProfile = false
children:
receiver =
SelfNode
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ ast: |
methodName = "foo"
notEmptyKeywordsProfile = false
notRuby2KeywordsHashProfile = false
ruby2KeywordsHashProfile = false
children:
receiver =
SelfNode
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@ ast: |
methodName = "bar"
notEmptyKeywordsProfile = false
notRuby2KeywordsHashProfile = false
ruby2KeywordsHashProfile = false
children:
receiver =
SelfNode
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,6 @@ ast: |
methodName = "bar"
notEmptyKeywordsProfile = false
notRuby2KeywordsHashProfile = false
ruby2KeywordsHashProfile = false
children:
receiver =
SelfNode
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@ ast: |
methodName = "bar"
notEmptyKeywordsProfile = false
notRuby2KeywordsHashProfile = false
ruby2KeywordsHashProfile = false
children:
receiver =
SelfNode
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@ ast: |
methodName = "convert"
notEmptyKeywordsProfile = false
notRuby2KeywordsHashProfile = false
ruby2KeywordsHashProfile = false
children:
arguments = [
IntegerFixnumLiteralNode
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@ ast: |
methodName = "convert"
notEmptyKeywordsProfile = false
notRuby2KeywordsHashProfile = false
ruby2KeywordsHashProfile = false
children:
arguments = [
IntegerFixnumLiteralNode
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,6 @@ ast: |
methodName = "foo="
notEmptyKeywordsProfile = false
notRuby2KeywordsHashProfile = false
ruby2KeywordsHashProfile = false
children:
arguments = [
DeadNode
Expand All @@ -97,7 +96,6 @@ ast: |
methodName = "a"
notEmptyKeywordsProfile = false
notRuby2KeywordsHashProfile = false
ruby2KeywordsHashProfile = false
children:
receiver =
SelfNode
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,6 @@ ast: |
methodName = "a"
notEmptyKeywordsProfile = false
notRuby2KeywordsHashProfile = false
ruby2KeywordsHashProfile = false
children:
receiver =
SelfNode
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ ast: |
methodName = "bar"
notEmptyKeywordsProfile = false
notRuby2KeywordsHashProfile = false
ruby2KeywordsHashProfile = false
children:
receiver =
SelfNode
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ ast: |
methodName = "convert"
notEmptyKeywordsProfile = false
notRuby2KeywordsHashProfile = false
ruby2KeywordsHashProfile = false
children:
arguments = [
IntegerFixnumLiteralNode
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ ast: |
methodName = "convert"
notEmptyKeywordsProfile = false
notRuby2KeywordsHashProfile = false
ruby2KeywordsHashProfile = false
children:
arguments = [
IntegerFixnumLiteralNode
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ ast: |
methodName = "`"
notEmptyKeywordsProfile = false
notRuby2KeywordsHashProfile = false
ruby2KeywordsHashProfile = false
children:
arguments = [
StringLiteralNode
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ ast: |
methodName = "`"
notEmptyKeywordsProfile = false
notRuby2KeywordsHashProfile = false
ruby2KeywordsHashProfile = false
children:
arguments = [
InterpolatedStringNode
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ ast: |
methodName = "`"
notEmptyKeywordsProfile = false
notRuby2KeywordsHashProfile = false
ruby2KeywordsHashProfile = false
children:
arguments = [
InterpolatedStringNode
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ public Object execute(VirtualFrame frame) {
final Object[] rubyArgs = RubyArguments.repack(frame.getArguments(), receiver, 0, 1);
RubyArguments.setArgument(rubyArgs, 0, methodSymbol);

return toEnumNode.execute(null, receiver, "to_enum", rubyArgs, PRIVATE, null);
return toEnumNode.execute(null, receiver, "to_enum", rubyArgs, PRIVATE);
} else {
return method.execute(frame);
}
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/org/truffleruby/cext/CExtNodes.java
Original file line number Diff line number Diff line change
Expand Up @@ -1244,7 +1244,7 @@ Object callSuper(VirtualFrame frame, Object[] args,
final InternalMethod superMethod = superMethodLookup.getMethod();
// This C API only passes positional arguments, but maybe it should be influenced by ruby2_keywords hashes?
return callSuperMethodNode.execute(
frame, callingSelf, superMethod, EmptyArgumentsDescriptor.INSTANCE, args, nil, null);
frame, callingSelf, superMethod, EmptyArgumentsDescriptor.INSTANCE, args, nil);
}

@TruffleBoundary
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -398,8 +398,7 @@ Object instanceExec(VirtualFrame frame, Object receiver, Object[] arguments, Rub
new SingletonClassOfSelfDefaultDefinee(receiver),
block.declarationContext.getRefinements());
var descriptor = RubyArguments.getDescriptor(frame);
return callBlockNode.executeCallBlock(
declarationContext, block, receiver, nil, descriptor, arguments, null);
return callBlockNode.executeCallBlock(declarationContext, block, receiver, nil, descriptor, arguments);
}

@Specialization
Expand All @@ -422,8 +421,7 @@ Object instanceExec(ArgumentsDescriptor descriptor, Object self, Object[] argume
new SingletonClassOfSelfDefaultDefinee(self),
block.declarationContext.getRefinements());

return callBlockNode.executeCallBlock(
declarationContext, block, self, nil, descriptor, arguments, null);
return callBlockNode.executeCallBlock(declarationContext, block, self, nil, descriptor, arguments);
}

}
Expand Down Expand Up @@ -561,7 +559,7 @@ Object send(Frame callerFrame, Object self, Object[] rubyArgs, RootCallTarget ta
@Cached NameToJavaStringNode nameToJavaString) {
Object name = RubyArguments.getArgument(rubyArgs, 0);
return dispatchNode.execute(callerFrame, self, nameToJavaString.execute(this, name),
RubyArguments.repack(rubyArgs, self, 1), PRIVATE, null);
RubyArguments.repack(rubyArgs, self, 1), PRIVATE);
}
}

Expand Down
3 changes: 1 addition & 2 deletions src/main/java/org/truffleruby/core/kernel/KernelNodes.java
Original file line number Diff line number Diff line change
Expand Up @@ -1396,8 +1396,7 @@ Object send(Frame callerFrame, Object self, Object[] rubyArgs, RootCallTarget ta
@Cached NameToJavaStringNode nameToJavaString) {
Object name = RubyArguments.getArgument(rubyArgs, 0);
Object[] newArgs = RubyArguments.repack(rubyArgs, self, 1);
return dispatchNode.execute(callerFrame, self, nameToJavaString.execute(this, name), newArgs, PUBLIC,
null);
return dispatchNode.execute(callerFrame, self, nameToJavaString.execute(this, name), newArgs, PUBLIC);
}

}
Expand Down
3 changes: 1 addition & 2 deletions src/main/java/org/truffleruby/core/klass/ClassNodes.java
Original file line number Diff line number Diff line change
Expand Up @@ -288,8 +288,7 @@ Object newInstance(Frame callerFrame, RubyClass rubyClass, Object[] rubyArgs, Ro
@Cached DispatchNode allocateNode,
@Cached DispatchNode initializeNode) {
final Object instance = allocateNode.call(rubyClass, "__allocate__");
initializeNode.execute(null, instance, "initialize", RubyArguments.repack(rubyArgs, instance), PRIVATE,
null);
initializeNode.execute(null, instance, "initialize", RubyArguments.repack(rubyArgs, instance), PRIVATE);
return instance;
}

Expand Down
2 changes: 1 addition & 1 deletion src/main/java/org/truffleruby/core/method/MethodNodes.java
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ static Object callBoundMethod(Frame frame, InternalMethod internalMethod, Object
final Object[] newArgs = RubyArguments.repack(callerRubyArgs, receiver);
RubyArguments.setMethod(newArgs, internalMethod);
assert RubyArguments.assertFrameArguments(newArgs);
return callInternalMethodNode.execute(frame, internalMethod, receiver, newArgs, null);
return callInternalMethodNode.execute(frame, internalMethod, receiver, newArgs);
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/main/java/org/truffleruby/core/method/RubyMethod.java
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ public Object execute(Object[] arguments,
final Object[] convertedArguments = foreignToRubyArgumentsNode.executeConvert(arguments);
final Object[] frameArgs = RubyArguments.pack(null, null, method, null, receiver, nil,
EmptyArgumentsDescriptor.INSTANCE, convertedArguments);
return callInternalMethodNode.execute(null, method, receiver, frameArgs, null);
return callInternalMethodNode.execute(null, method, receiver, frameArgs);
}
// endregion

Expand Down
5 changes: 2 additions & 3 deletions src/main/java/org/truffleruby/core/module/ModuleNodes.java
Original file line number Diff line number Diff line change
Expand Up @@ -808,7 +808,7 @@ Object classExec(ArgumentsDescriptor descriptor, RubyModule self, Object[] args,
new FixedDefaultDefinee(self),
block.declarationContext.getRefinements());

return callBlockNode.executeCallBlock(declarationContext, block, self, nil, descriptor, args, null);
return callBlockNode.executeCallBlock(declarationContext, block, self, nil, descriptor, args);
}
}

Expand Down Expand Up @@ -2268,8 +2268,7 @@ RubyModule refine(RubyModule namespace, RubyModule moduleToRefine, RubyProc bloc
refinement,
nil,
EmptyArgumentsDescriptor.INSTANCE,
EMPTY_ARGUMENTS,
null);
EMPTY_ARGUMENTS);
return refinement;
}

Expand Down
3 changes: 1 addition & 2 deletions src/main/java/org/truffleruby/core/proc/ProcNodes.java
Original file line number Diff line number Diff line change
Expand Up @@ -166,8 +166,7 @@ Object call(Frame callerFrame, RubyProc proc, Object[] rubyArgs, RootCallTarget
ProcOperations.getSelf(proc),
RubyArguments.getBlock(rubyArgs),
RubyArguments.getDescriptor(rubyArgs),
RubyArguments.getRawArguments(rubyArgs),
null);
RubyArguments.getRawArguments(rubyArgs));
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,28 +35,18 @@
public abstract class DispatchMethodMissingNode extends RubyBaseNode {

public abstract Object execute(Frame frame, Object receiver, String methodName, Object[] rubyArgs,
DispatchConfiguration config, LiteralCallNode literalCallNode);
DispatchConfiguration config);

@Specialization(guards = "config.missingBehavior == RETURN_MISSING")
static Object dispatchReturnMissing(
Frame frame,
Object receiver,
String methodName,
Object[] rubyArgs,
DispatchConfiguration config,
LiteralCallNode literalCallNode) {
Frame frame, Object receiver, String methodName, Object[] rubyArgs, DispatchConfiguration config) {
return MISSING;
}

@InliningCutoff
@Specialization(guards = { "config.missingBehavior == CALL_METHOD_MISSING", "isForeignObject(receiver)" })
static Object dispatchForeign(
Frame frame,
Object receiver,
String methodName,
Object[] rubyArgs,
DispatchConfiguration config,
LiteralCallNode literalCallNode,
Frame frame, Object receiver, String methodName, Object[] rubyArgs, DispatchConfiguration config,
@Cached CallForeignMethodNode callForeign) {
final Object block = RubyArguments.getBlock(rubyArgs);
final Object[] arguments = RubyArguments.getPositionalArguments(rubyArgs);
Expand All @@ -66,12 +56,7 @@ static Object dispatchForeign(
@InliningCutoff
@Specialization(guards = { "config.missingBehavior == CALL_METHOD_MISSING", "!isForeignObject(receiver)" })
static Object dispatchMissingMethod(
Frame frame,
Object receiver,
String methodName,
Object[] rubyArgs,
DispatchConfiguration config,
LiteralCallNode literalCallNode,
Frame frame, Object receiver, String methodName, Object[] rubyArgs, DispatchConfiguration config,
@Cached ToSymbolNode toSymbol,
@Cached DispatchNode callMethodMissing,
@Cached InlinedBranchProfile methodMissingMissingProfile,
Expand All @@ -82,7 +67,7 @@ static Object dispatchMissingMethod(

RubyArguments.setArgument(newArgs, 0, symbolName);
final Object result = callMethodMissing.execute(frame, receiver, "method_missing", newArgs,
DispatchConfiguration.PRIVATE_RETURN_MISSING_IGNORE_REFINEMENTS, literalCallNode);
DispatchConfiguration.PRIVATE_RETURN_MISSING_IGNORE_REFINEMENTS);

if (result == MISSING) {
methodMissingMissingProfile.enter(node);
Expand Down
Loading
Loading