From 4f473905e14a7dbfdd29248f7a0673022dfaa699 Mon Sep 17 00:00:00 2001 From: normal Date: Tue, 18 Jul 2017 04:30:08 +0000 Subject: [PATCH] newhash insn reuses existing keys This gives the newhash VM instruction the same string reuse capabilities as rb_hash_aset. * st.c (str_key): new wrapper function to call rb_fstring_existing (rb_hash_bulk_insert): use str_key * test/ruby/test_optimization.rb (test_hash_reuse_fstring): ensure key reuse for newhash instructions git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/trunk@59355 b2dd03c8-39d4-4d8f-98ff-823fe69b080e --- st.c | 20 ++++++++++++++++++-- test/ruby/test_optimization.rb | 7 +++++++ 2 files changed, 25 insertions(+), 2 deletions(-) diff --git a/st.c b/st.c index c2848b69b6..c983c0291b 100644 --- a/st.c +++ b/st.c @@ -2092,6 +2092,23 @@ st_rehash(st_table *tab) } #ifdef RUBY + +static VALUE +str_key(VALUE key) +{ + VALUE k; + + if (RB_OBJ_FROZEN(key)) { + return key; + } + if ((k = rb_fstring_existing(key)) != Qnil) { + return k; + } + else { + return rb_str_new_frozen(key); + } +} + /* Mimics ruby's { foo => bar } syntax. This function is placed here because it touches table internals and write barriers at once. */ void @@ -2114,8 +2131,7 @@ rb_hash_bulk_insert(long argc, const VALUE *argv, VALUE hash) for (i = 0; i < argc; /* */) { VALUE key = argv[i++]; VALUE val = argv[i++]; - st_data_t k = (rb_obj_class(key) == rb_cString) ? - rb_str_new_frozen(key) : key; + st_data_t k = (rb_obj_class(key) == rb_cString) ? str_key(key) : key; st_table_entry e; e.hash = do_hash(k, tab); e.key = k; diff --git a/test/ruby/test_optimization.rb b/test/ruby/test_optimization.rb index 0d14c9c932..6cf6c63d1b 100644 --- a/test/ruby/test_optimization.rb +++ b/test/ruby/test_optimization.rb @@ -196,6 +196,9 @@ class TestRubyOptimization < Test::Unit::TestCase assert_predicate h.keys[0], :frozen? assert_same exp, h.keys[0] + h = { key => 1 } + assert_same exp, h.keys[0], 'newhash insn should reuse strings, too' + h1 = {} h2 = {} key.taint @@ -206,6 +209,10 @@ class TestRubyOptimization < Test::Unit::TestCase assert_same k1, k2 assert_predicate k1, :tainted? + h = { key => 1 } + assert_not_predicate key, :frozen? + assert_same k1, h.keys[0], 'newhash insn should reuse tainted strings' + assert_equal GC::INTERNAL_CONSTANTS[:RVALUE_SIZE], ObjectSpace.memsize_of(k1), 'tainted string should share with untainted fstring'