Skip to content

Commit b01678d

Browse files
committed
[GR-37597] Avoid an extra copy of a Ruby array and add more profiles for splat calls
PullRequest: truffleruby/3256
2 parents 5b43516 + 263a622 commit b01678d

File tree

4 files changed

+25
-7
lines changed

4 files changed

+25
-7
lines changed

bench/micro/dispatch/dispatch-mono-splat.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
class Callee
2-
def call(arg1 = nil, arg2 = nil, arg3 = nil, arg4 = nil)
2+
def call(arg1 = nil, arg2 = nil, arg3 = nil)
33
:foo
44
end
55
end

src/main/java/org/truffleruby/language/dispatch/LiteralCallNode.java

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,8 @@ public abstract class LiteralCallNode extends RubyContextSourceNode {
2727
@Child private CopyHashAndSetRuby2KeywordsNode copyHashAndSetRuby2KeywordsNode;
2828

2929
protected final boolean isSplatted;
30-
@CompilationFinal private boolean notRuby2KeywordsHashProfile, emptyKeywordsProfile, notEmptyKeywordsProfile;
30+
@CompilationFinal private boolean lastArgIsNotHashProfile, notRuby2KeywordsHashProfile, emptyKeywordsProfile,
31+
notEmptyKeywordsProfile;
3132

3233
protected LiteralCallNode(boolean isSplatted, ArgumentsDescriptor descriptor) {
3334
this.isSplatted = isSplatted;
@@ -42,7 +43,17 @@ protected ArgumentsDescriptor getArgumentsDescriptorAndCheckRuby2KeywordsHash(Ob
4243
if (userArgsCount > 0) {
4344
final Object lastArgument = ArrayUtils.getLast(args);
4445
assert lastArgument != null;
45-
if (isRuby2KeywordsHash(lastArgument)) { // both branches profiled
46+
47+
if (!(lastArgument instanceof RubyHash)) {
48+
if (!lastArgIsNotHashProfile) {
49+
CompilerDirectives.transferToInterpreterAndInvalidate();
50+
lastArgIsNotHashProfile = true;
51+
}
52+
53+
return descriptor;
54+
}
55+
56+
if (((RubyHash) lastArgument).ruby2_keywords) { // both branches profiled
4657
if (copyHashAndSetRuby2KeywordsNode == null) {
4758
CompilerDirectives.transferToInterpreterAndInvalidate();
4859
copyHashAndSetRuby2KeywordsNode = insert(CopyHashAndSetRuby2KeywordsNode.create());
@@ -63,12 +74,10 @@ protected ArgumentsDescriptor getArgumentsDescriptorAndCheckRuby2KeywordsHash(Ob
6374
return descriptor;
6475
}
6576

66-
private boolean isRuby2KeywordsHash(Object lastArgument) {
67-
return lastArgument instanceof RubyHash && ((RubyHash) lastArgument).ruby2_keywords;
68-
}
69-
7077
// NOTE: args is either frame args or user args
7178
protected boolean emptyKeywordArguments(Object[] args) {
79+
assert isSplatted || descriptor instanceof KeywordArgumentsDescriptor;
80+
7281
if (((RubyHash) ArrayUtils.getLast(args)).empty()) {
7382
if (!emptyKeywordsProfile) {
7483
CompilerDirectives.transferToInterpreterAndInvalidate();

src/main/java/org/truffleruby/language/exceptions/RescueSplatNode.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ public RescueSplatNode(RubyLanguage language, RubyNode handlingClassesArray, Rub
3535
SplatCastNode.NilBehavior.EMPTY_ARRAY,
3636
false,
3737
handlingClassesArray);
38+
this.splatCastNode.doNotCopy();
3839
this.stores = ArrayStoreLibrary.getFactory().createDispatched(ArrayGuards.storageStrategyLimit());
3940
}
4041

src/main/java/org/truffleruby/parser/BodyTranslator.java

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -700,6 +700,14 @@ protected ArgumentsAndBlockTranslation translateArgumentsAndBlock(SourceIndexLen
700700
argumentsTranslated[i] = arguments[i].accept(this);
701701
}
702702

703+
if (isSplatted) {
704+
assert argumentsTranslated.length == 1;
705+
// No need to copy the array for call(*splat), the elements will be copied to the frame arguments
706+
if (argumentsTranslated[0] instanceof SplatCastNode) {
707+
((SplatCastNode) argumentsTranslated[0]).doNotCopy();
708+
}
709+
}
710+
703711
ParseNode blockPassNode = null;
704712
if (iterNode instanceof BlockPassParseNode) {
705713
blockPassNode = ((BlockPassParseNode) iterNode).getBodyNode();

0 commit comments

Comments
 (0)