From 4f6a890180558503305222284a4aa7a50778bda6 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Fri, 25 Aug 2017 11:54:20 -0700 Subject: [PATCH] Reduce Array object allocations when querying PG MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This patch reduced the number of array allocations made when querying PG. It works by returning a PG specific subclass of `ActiveRecord::Result` that delegates to the `PGResult` class. This allows us to iterate over the result rows only once (or possibly even lazily). I removed a call to `PGResult#clear`. I believe this is a safe change because the PGResult free function will clear the results when the object gets garbage collected: https://github.com/ged/ruby-pg/blob/6e6a44b5bfc1ff5c4147dcf45448067e0fe07567/ext/pg_result.c#L211-L212 I don't think we need to aggressively clear the result. Here is the benchmark I used: ```ruby require 'active_record' require 'benchmark/ips' require 'allocation_tracer' ActiveRecord::Base.establish_connection(adapter: "postgresql") ActiveRecord::Migration.verbose = false ActiveRecord::Schema.define do create_table :users, force: true do |t| t.string :name, :email t.timestamps null: false end end class User < ActiveRecord::Base; end attributes = { name: "Lorem ipsum dolor sit amet, consectetur adipiscing elit.", email: "foobar@email.com" } 1000.times do User.create!(attributes) end ObjectSpace::AllocationTracer.setup(%i{type}) result = ObjectSpace::AllocationTracer.trace do str = "" User.all.each do |user| str << "name: #{user.name} email: #{user.email}\n" end end p :ALLOCATIONS => result p :TOTAL_ALLOCATIONS => result.map { |_,v| v.first }.inject(:+) Benchmark.ips do |x| x.report("all") do str = "" User.all.each do |user| str << "name: #{user.name} email: #{user.email}\n" end end end ``` Here are the results from BEFORE: ``` {:ALLOCATIONS=>{[:T_STRING]=>[8018, 0, 0, 0, 0, 0], [:T_ARRAY]=>[1068, 0, 0, 0, 0, 0], [:T_OBJECT]=>[5011, 0, 0, 0, 0, 0], [:T_HASH]=>[6009, 0, 0, 0, 0, 0], [:T_DATA]=>[12, 0, 0, 0, 0, 0], [:T_IMEMO]=>[24, 0, 0, 0, 0, 0], [:T_STRUCT]=>[1, 0, 0, 0, 0, 0]}} {:TOTAL_ALLOCATIONS=>20143} Warming up -------------------------------------- all 5.000 i/100ms Calculating ------------------------------------- all 60.787 (± 3.3%) i/s - 305.000 in 5.021694s ``` and AFTER: ``` {:ALLOCATIONS=>{[:T_STRING]=>[8013, 0, 1169, 0, 1, 0], [:T_ARRAY]=>[65, 0, 60, 0, 1, 0], [:T_OBJECT]=>[5011, 2, 878, 0, 1, 0], [:T_HASH]=>[6010, 0, 1741, 0, 1, 0], [:T_DATA]=>[12, 2, 12, 1, 1, 0], [:T_IMEMO]=>[24, 4, 20, 0, 1, 0], [:T_STRUCT]=>[1, 0, 1, 1, 1, 0]}} {:TOTAL_ALLOCATIONS=>19136} Warming up -------------------------------------- all 6.000 i/100ms Calculating ------------------------------------- all 63.746 (± 3.1%) i/s - 324.000 in 5.087608s ``` --- .../postgresql/database_statements.rb | 42 ++++++++++++++++++- .../connection_adapters/postgresql_adapter.rb | 1 - activerecord/lib/active_record/result.rb | 7 ++-- 3 files changed, 45 insertions(+), 5 deletions(-) diff --git a/activerecord/lib/active_record/connection_adapters/postgresql/database_statements.rb b/activerecord/lib/active_record/connection_adapters/postgresql/database_statements.rb index 8db2a645af..b875249bd6 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql/database_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql/database_statements.rb @@ -77,6 +77,46 @@ def execute(sql, name = nil) end end + class PGResult < ActiveRecord::Result + def initialize(result, types = {}) + @result = result + # Instances of this should never access the `@rows` ivar. + super(nil, nil, types) + end + + def length + @result.ntuples + end + + def rows + @result.values + end + + def columns + @result.fields + end + + # Returns the first record from the rows collection. + # If the rows collection is empty, returns +nil+. + def first + return nil if empty? + @result.first + end + + # Returns the last record from the rows collection. + # If the rows collection is empty, returns +nil+. + def last + return nil if empty? + @result.last + end + + private + + def hash_rows + @result + end + end + def exec_query(sql, name = "SQL", binds = [], prepare: false) execute_and_clear(sql, name, binds, prepare: prepare) do |result| types = {} @@ -86,7 +126,7 @@ def exec_query(sql, name = "SQL", binds = [], prepare: false) fmod = result.fmod i types[fname] = get_oid_type(ftype, fmod, fname) end - ActiveRecord::Result.new(fields, result.values, types) + PGResult.new(result, types) end end diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb index 4d37a292d6..7e63940e96 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb @@ -576,7 +576,6 @@ def execute_and_clear(sql, name, binds, prepare: false) result = exec_cache(sql, name, binds) end ret = yield result - result.clear ret end diff --git a/activerecord/lib/active_record/result.rb b/activerecord/lib/active_record/result.rb index e54e8086dd..cf3731a56a 100644 --- a/activerecord/lib/active_record/result.rb +++ b/activerecord/lib/active_record/result.rb @@ -56,13 +56,13 @@ def each if block_given? hash_rows.each { |row| yield row } else - hash_rows.to_enum { @rows.size } + hash_rows.to_enum { length } end end # Returns an array of hashes representing each row record. def to_hash - hash_rows + hash_rows.to_a end alias :map! :map @@ -70,7 +70,7 @@ def to_hash # Returns true if there are no records, otherwise false. def empty? - rows.empty? + length == 0 end # Returns an array of hashes representing each row record. @@ -110,6 +110,7 @@ def initialize_copy(other) @rows = rows.dup @column_types = column_types.dup @hash_rows = nil + super end private -- GitLab