From 693c66dfde891a5d540dc4cdc0c712495c31100c Mon Sep 17 00:00:00 2001 From: David Yip Date: Sun, 15 Oct 2017 02:32:03 -0500 Subject: [PATCH] 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) --- app/models/keyword_mute.rb | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/app/models/keyword_mute.rb b/app/models/keyword_mute.rb index e1a8c3712c..f94e0f7951 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