Skip to content

Commit 6446003

Browse files
committed
[GR-26646] Avoid dispatch to #hash method for Hash lookups where compatible
PullRequest: truffleruby/2065
2 parents 3c7e171 + e91a12c commit 6446003

20 files changed

+332
-125
lines changed

spec/ruby/core/hash/element_reference_spec.rb

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -117,4 +117,18 @@
117117
key = HashSpecs::KeyWithPrivateHash.new
118118
{ key => 42 }[key].should == 42
119119
end
120+
121+
it "does not dispatch to hash for Boolean, Integer, Float, String, or Symbol" do
122+
code = <<-EOC
123+
load '#{fixture __FILE__, "name.rb"}'
124+
hash = { true => 42, false => 42, 1 => 42, 2.0 => 42, "hello" => 42, :ok => 42 }
125+
[true, false, 1, 2.0, "hello", :ok].each do |value|
126+
raise "incorrect value" unless hash[value] == 42
127+
end
128+
puts "Ok."
129+
EOC
130+
result = ruby_exe(code, args: "2>&1")
131+
result.should == "Ok.\n"
132+
end
133+
120134
end

spec/ruby/core/hash/fixtures/name.rb

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
class TrueClass
2+
def hash
3+
raise "TrueClass#hash should not be called"
4+
end
5+
end
6+
class FalseClass
7+
def hash
8+
raise "FalseClass#hash should not be called"
9+
end
10+
end
11+
class Integer
12+
def hash
13+
raise "Integer#hash should not be called"
14+
end
15+
end
16+
class Float
17+
def hash
18+
raise "Float#hash should not be called"
19+
end
20+
end
21+
class String
22+
def hash
23+
raise "String#hash should not be called"
24+
end
25+
end
26+
class Symbol
27+
def hash
28+
raise "Symbol#hash should not be called"
29+
end
30+
end

spec/ruby/core/hash/shared/store.rb

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,4 +95,21 @@ def key.reverse() "bar" end
9595
hash.each { hash.send(@method, 1, :foo) }
9696
hash.should == {1 => :foo, 3 => 4, 5 => 6}
9797
end
98+
99+
it "does not dispatch to hash for Boolean, Integer, Float, String, or Symbol" do
100+
code = <<-EOC
101+
load '#{fixture __FILE__, "name.rb"}'
102+
hash = {}
103+
[true, false, 1, 2.0, "hello", :ok].each do |value|
104+
hash[value] = 42
105+
raise "incorrect value" unless hash[value] == 42
106+
hash[value] = 43
107+
raise "incorrect value" unless hash[value] == 43
108+
end
109+
puts "OK"
110+
puts hash.size
111+
EOC
112+
result = ruby_exe(code, args: "2>&1")
113+
result.should == "OK\n6\n"
114+
end
98115
end
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
slow:Hash#[] does not dispatch to hash for Boolean, Integer, Float, String, or Symbol
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
slow:Hash#[]= does not dispatch to hash for Boolean, Integer, Float, String, or Symbol

spec/tags/core/hash/store_tags.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
slow:Hash#store does not dispatch to hash for Boolean, Integer, Float, String, or Symbol

spec/truffle/launcher_spec.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -315,7 +315,7 @@ def should_print_full_java_command(options, env: {})
315315
out = ruby_exe("puts 14.hash", options: "--experimental-options --hashing-deterministic", args: "2>&1")
316316
check_status_or_print(out)
317317
out.should include("SEVERE: deterministic hashing is enabled - this may make you vulnerable to denial of service attacks")
318-
out.should include("7141275149799654099")
318+
out.should include("3412061130696242594")
319319
end
320320

321321
it "prints help containing runtime options" do

src/main/java/org/truffleruby/RubyContext.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@
6262
import org.truffleruby.interop.InteropManager;
6363
import org.truffleruby.language.CallStackManager;
6464
import org.truffleruby.language.LexicalScope;
65-
import org.truffleruby.language.RubyNode;
65+
import org.truffleruby.language.RubyBaseNode;
6666
import org.truffleruby.language.SafepointManager;
6767
import org.truffleruby.language.arguments.RubyArguments;
6868
import org.truffleruby.language.backtrace.BacktraceFormatter;
@@ -524,7 +524,7 @@ public TruffleLanguage.Env getEnv() {
524524
}
525525

526526
/** Hashing for a RubyNode, the seed should only be used for a Ruby-level #hash method */
527-
public Hashing getHashing(RubyNode node) {
527+
public Hashing getHashing(RubyBaseNode node) {
528528
return hashing;
529529
}
530530

src/main/java/org/truffleruby/cext/CExtNodes.java

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@
3838
import org.truffleruby.core.encoding.RubyEncoding;
3939
import org.truffleruby.core.exception.ErrnoErrorNode;
4040
import org.truffleruby.core.exception.ExceptionOperations;
41-
import org.truffleruby.core.hash.HashNode;
41+
import org.truffleruby.core.hash.HashingNodes;
4242
import org.truffleruby.core.klass.RubyClass;
4343
import org.truffleruby.core.module.MethodLookupResult;
4444
import org.truffleruby.core.module.ModuleNodes.ConstSetNode;
@@ -945,16 +945,10 @@ protected Object rbSysErrFail(int errno, RubyString message,
945945
@CoreMethod(names = "rb_hash", onSingleton = true, required = 1)
946946
public abstract static class RbHashNode extends CoreMethodArrayArgumentsNode {
947947

948-
@Child private HashNode hash;
949-
950948
@Specialization
951-
protected Object rbHash(Object object) {
952-
if (hash == null) {
953-
CompilerDirectives.transferToInterpreterAndInvalidate();
954-
hash = insert(new HashNode());
955-
}
956-
957-
return hash.hash(object, false);
949+
protected int rbHash(Object object,
950+
@Cached HashingNodes.ToHashByHashCode toHashByHashCode) {
951+
return toHashByHashCode.execute(object);
958952
}
959953
}
960954

src/main/java/org/truffleruby/core/hash/HashLiteralNode.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ public Object execute(VirtualFrame frame) {
6464

6565
public static class SmallHashLiteralNode extends HashLiteralNode {
6666

67-
@Child private HashNode hashNode;
67+
@Child private HashingNodes.ToHashByHashCode hashNode;
6868
@Child private DispatchNode equalNode;
6969
@Child private BooleanCastNode booleanCastNode;
7070
@Child private FreezeHashKeyIfNeededNode freezeHashKeyIfNeededNode = FreezeHashKeyIfNeededNodeGen.create();
@@ -124,9 +124,9 @@ public Object execute(VirtualFrame frame) {
124124
private int hash(Object key) {
125125
if (hashNode == null) {
126126
CompilerDirectives.transferToInterpreterAndInvalidate();
127-
hashNode = insert(new HashNode());
127+
hashNode = insert(HashingNodes.ToHashByHashCode.create());
128128
}
129-
return hashNode.hash(key, false);
129+
return hashNode.execute(key);
130130
}
131131

132132
private boolean callEqual(Object receiver, Object key) {

0 commit comments

Comments
 (0)