about summary refs log tree commit diff
diff options
context:
space:
mode:
authorDavid Yip <yipdw@member.fsf.org>2017-10-15 02:32:03 -0500
committerDavid Yip <yipdw@member.fsf.org>2017-10-21 14:54:36 -0500
commit693c66dfde891a5d540dc4cdc0c712495c31100c (patch)
treef39cea2804dd4a3c41f1cbb29e5978f1730f0b68
parenta4851100fd3f20fdc01a3ebf7942fab6d5d03ebf (diff)
Use more idiomatic string concatentation. #164.
The intent of the previous concatenation was to minimize object
allocations, which can end up being a slow killer.  However, it turns
out that under MRI 2.4.x, the shove-strings-in-an-array-and-join method
is not only arguably more common but (in this particular case) actually
allocates *fewer* objects than the string concatenation.

Or, at least, that's what I gather by running this:

    words = %w(palmettoes nudged hibernation bullish stockade's tightened Hades
    Dixie's formalize superego's commissaries Zappa's viceroy's apothecaries
    tablespoonful's barons Chennai tollgate ticked expands)

    a = Account.first

    KeywordMute.transaction do
      words.each { |w| KeywordMute.create!(keyword: w, account: a) }

      GC.start

      s1 = GC.stat

      re = String.new.tap do |str|
        scoped = KeywordMute.where(account: a)
        keywords = scoped.select(:id, :keyword)
        count = scoped.count

        keywords.find_each.with_index do |kw, index|
          str << Regexp.escape(kw.keyword.strip)
          str << '|' if index < count - 1
        end
      end

      s2 = GC.stat

      puts s1.inspect, s2.inspect

      raise ActiveRecord::Rollback
    end

vs this:

    words = %w( palmettoes nudged hibernation bullish stockade's tightened Hades Dixie's
    formalize superego's commissaries Zappa's viceroy's apothecaries tablespoonful's
    barons Chennai tollgate ticked expands
    )

    a = Account.first

    KeywordMute.transaction do
      words.each { |w| KeywordMute.create!(keyword: w, account: a) }

      GC.start

      s1 = GC.stat

      re = [].tap do |arr|
        KeywordMute.where(account: a).select(:keyword, :id).find_each do |m|
          arr << Regexp.escape(m.keyword.strip)
        end
      end.join('|')

      s2 = GC.stat

      puts s1.inspect, s2.inspect

      raise ActiveRecord::Rollback
    end

Using rails r, here is a comparison of the total_allocated_objects and
malloc_increase_bytes GC stat data:

                 total_allocated_objects        malloc_increase_bytes
string concat    3200241 -> 3201428 (+1187)     1176 -> 45216 (44040)
array join       3200380 -> 3201299 (+919)      1176 -> 36448 (35272)
-rw-r--r--app/models/keyword_mute.rb13
1 files changed, 4 insertions, 9 deletions
diff --git a/app/models/keyword_mute.rb b/app/models/keyword_mute.rb
index e1a8c3712..f94e0f795 100644
--- a/app/models/keyword_mute.rb
+++ b/app/models/keyword_mute.rb
@@ -23,16 +23,11 @@ class KeywordMute < ApplicationRecord
     attr_reader :regex
 
     def initialize(account)
-      re = String.new.tap do |str|
-        scoped = KeywordMute.where(account: account)
-        keywords = scoped.select(:id, :keyword)
-        count = scoped.count
-
-        keywords.find_each.with_index do |kw, index|
-          str << Regexp.escape(kw.keyword.strip)
-          str << '|' if index < count - 1
+      re = [].tap do |arr|
+        KeywordMute.where(account: account).select(:keyword, :id).find_each do |m|
+          arr << Regexp.escape(m.keyword.strip)
         end
-      end
+      end.join('|')
 
       @regex = /\b(?:#{re})\b/i unless re.empty?
     end