Skip to content

Commit

Permalink
Increase speed of Model#this by about 85%
Browse files Browse the repository at this point in the history
Previously, this was doing 3 dataset clones.  Add a
Model.instance_dataset method that returns a cached intermediate
dataset, so that Model#this only has to do one dataset clone.

Quite a bit of spec fallout since the specs assumed that you could
modify Model.dataset when you now need to modify
Model.instance_dataset.
  • Loading branch information
jeremyevans committed Mar 26, 2012
1 parent 60daa68 commit 8accebb
Show file tree
Hide file tree
Showing 15 changed files with 61 additions and 47 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG
@@ -1,5 +1,7 @@
=== HEAD

* Increase speed of Model#this by about 85% (jeremyevans)

* Increase speed of Model#delete and #destroy by about 75% for models with simple datasets (jeremyevans)

* Make nested_attributes plugin work when destroying/removing associated objects when strict_param_setting is true (r-stu31) (#455)
Expand Down
3 changes: 2 additions & 1 deletion lib/sequel/model.rb
Expand Up @@ -113,7 +113,8 @@ class Model
:@simple_pk=>nil, :@simple_table=>nil, :@strict_param_setting=>nil,
:@typecast_empty_string_to_nil=>nil, :@typecast_on_assignment=>nil,
:@raise_on_typecast_failure=>nil, :@plugins=>:dup, :@setter_methods=>nil,
:@use_after_commit_rollback=>nil}
:@use_after_commit_rollback=>nil, :@fast_pk_lookup_sql=>nil,
:@fast_instance_delete_sql=>nil}

# Regular expression that determines if a method name is normal in the sense that
# it could be used literally in ruby code without using send. Used to
Expand Down
11 changes: 8 additions & 3 deletions lib/sequel/model/base.rb
Expand Up @@ -30,9 +30,13 @@ module ClassMethods
attr_reader :dataset_methods

# SQL string fragment used for faster DELETE statement creation when deleting/destroying
# model instances, or nil if the optimization should not be used.
# model instances, or nil if the optimization should not be used. For internal use only.
attr_reader :fast_instance_delete_sql

# The dataset that instance datasets (#this) are based on. Generally a naked version of
# the model's dataset limited to one row. For internal use only.
attr_reader :instance_dataset

# Array of plugin modules loaded by this class
#
# Sequel::Model.plugins
Expand Down Expand Up @@ -531,6 +535,7 @@ def set_dataset(ds, opts={})
end
@dataset.model = self if @dataset.respond_to?(:model=)
check_non_connection_error{@db_schema = (inherited ? superclass.db_schema : get_db_schema)}
@instance_dataset = @dataset.limit(1).naked
self
end

Expand Down Expand Up @@ -1372,7 +1377,7 @@ def singleton_method_added(meth)
# Artist[1].this
# # SELECT * FROM artists WHERE (id = 1) LIMIT 1
def this
@this ||= use_server(model.dataset.filter(pk_hash).limit(1).naked)
@this ||= use_server(model.instance_dataset.filter(pk_hash))
end

# Runs #set with the passed hash and then runs save_changes.
Expand Down Expand Up @@ -1517,7 +1522,7 @@ def _insert
# The dataset to use when inserting a new object. The same as the model's
# dataset by default.
def _insert_dataset
use_server(model.dataset)
use_server(model.instance_dataset)
end

# Insert into the given dataset and return the primary key created (if any).
Expand Down
4 changes: 2 additions & 2 deletions spec/extensions/class_table_inheritance_spec.rb
Expand Up @@ -130,10 +130,10 @@ class ::Executive < Manager; end
end

it "should lazily load attributes for columns in subclass tables" do
Manager.dataset._fetch = {:id=>1, :name=>'J', :kind=>'Executive', :num_staff=>2}
Manager.instance_dataset._fetch = Manager.dataset._fetch = {:id=>1, :name=>'J', :kind=>'Executive', :num_staff=>2}
m = Manager[1]
@db.sqls.should == ['SELECT * FROM employees INNER JOIN managers USING (id) WHERE (id = 1) LIMIT 1']
Executive.dataset._fetch = {:num_managers=>3}
Executive.instance_dataset._fetch = Executive.dataset._fetch = {:num_managers=>3}
m.num_managers.should == 3
@db.sqls.should == ['SELECT num_managers FROM employees INNER JOIN managers USING (id) INNER JOIN executives USING (id) WHERE (id = 1) LIMIT 1']
m.values.should == {:id=>1, :name=>'J', :kind=>'Executive', :num_staff=>2, :num_managers=>3}
Expand Down
6 changes: 4 additions & 2 deletions spec/extensions/force_encoding_spec.rb
Expand Up @@ -74,25 +74,27 @@

specify "should work when saving new model instances" do
o = @c.new
@c.dataset = ds = MODEL_DB[:a]
ds = MODEL_DB[:a]
def ds.first
s = 'blah'
s.force_encoding('US-ASCII')
{:id=>1, :x=>s}
end
@c.dataset = ds
o.save
o.x.should == 'blah'
o.x.encoding.should == @e1
end

specify "should work when refreshing model instances" do
o = @c.load(:id=>1, :x=>'as')
@c.dataset = ds = MODEL_DB[:a]
ds = MODEL_DB[:a]
def ds.first
s = 'blah'
s.force_encoding('US-ASCII')
{:id=>1, :x=>s}
end
@c.dataset = ds
o.refresh
o.x.should == 'blah'
o.x.encoding.should == @e1
Expand Down
4 changes: 2 additions & 2 deletions spec/extensions/lazy_attributes_spec.rb
Expand Up @@ -11,7 +11,7 @@ class ::LazyAttributesModel < Sequel::Model(@db[:la])
meta_def(:columns){[:id, :name]}
lazy_attributes :name
meta_def(:columns){[:id]}
dataset._fetch = proc do |sql|
instance_dataset._fetch = dataset._fetch = proc do |sql|
if sql !~ /WHERE/
if sql =~ /name/
[{:id=>1, :name=>'1'}, {:id=>2, :name=>'2'}]
Expand Down Expand Up @@ -133,7 +133,7 @@ def name

it "should work with the serialization plugin" do
@c.plugin :serialization, :yaml, :name
@ds._fetch = [[{:id=>1}, {:id=>2}], [{:id=>1, :name=>"--- 3\n"}, {:id=>2, :name=>"--- 6\n"}], [{:id=>1}], [{:name=>"--- 3\n"}]]
@c.instance_dataset._fetch = @ds._fetch = [[{:id=>1}, {:id=>2}], [{:id=>1, :name=>"--- 3\n"}, {:id=>2, :name=>"--- 6\n"}], [{:id=>1}], [{:name=>"--- 3\n"}]]
@c.with_identity_map do
ms = @ds.all
ms.map{|m| m.values}.should == [{:id=>1}, {:id=>2}]
Expand Down
8 changes: 4 additions & 4 deletions spec/extensions/list_spec.rb
Expand Up @@ -76,8 +76,8 @@ def klass(opts={})
end

it "should have position field set to max+1 when creating if not already set" do
@c.dataset._fetch = [[{:pos=>nil}], [{:id=>1, :position=>1}], [{:pos=>1}], [{:id=>2, :position=>2}]]
@c.dataset.autoid = 1
@c.instance_dataset._fetch = @c.dataset._fetch = [[{:pos=>nil}], [{:id=>1, :position=>1}], [{:pos=>1}], [{:id=>2, :position=>2}]]
@c.instance_dataset.autoid = @c.dataset.autoid = 1
@c.create.values.should == {:id=>1, :position=>1}
@c.create.values.should == {:id=>2, :position=>2}
@db.sqls.should == ["SELECT max(position) FROM items LIMIT 1",
Expand All @@ -89,8 +89,8 @@ def klass(opts={})
end

it "should have position field set to max+1 in scope when creating if not already set" do
@sc.dataset._fetch = [[{:pos=>nil}], [{:id=>1, :scope_id=>1, :position=>1}], [{:pos=>1}], [{:id=>2, :scope_id=>1, :position=>2}], [{:pos=>nil}], [{:id=>3, :scope_id=>2, :position=>1}]]
@sc.dataset.autoid = 1
@sc.instance_dataset._fetch = @sc.dataset._fetch = [[{:pos=>nil}], [{:id=>1, :scope_id=>1, :position=>1}], [{:pos=>1}], [{:id=>2, :scope_id=>1, :position=>2}], [{:pos=>nil}], [{:id=>3, :scope_id=>2, :position=>1}]]
@sc.instance_dataset.autoid = @sc.dataset.autoid = 1
@sc.create(:scope_id=>1).values.should == {:id=>1, :scope_id=>1, :position=>1}
@sc.create(:scope_id=>1).values.should == {:id=>2, :scope_id=>1, :position=>2}
@sc.create(:scope_id=>2).values.should == {:id=>3, :scope_id=>2, :position=>1}
Expand Down
4 changes: 2 additions & 2 deletions spec/extensions/optimistic_locking_spec.rb
Expand Up @@ -6,7 +6,7 @@
end
h = {1=>{:id=>1, :name=>'John', :lock_version=>2}}
lv = @lv = "lock_version"
@c.dataset.numrows = proc do |sql|
@c.instance_dataset.numrows = @c.dataset.numrows = proc do |sql|
case sql
when /UPDATE people SET (name|#{lv}) = ('Jim'|'Bob'|\d+), (?:name|#{lv}) = ('Jim'|'Bob'|\d+) WHERE \(\(id = (\d+)\) AND \(#{lv} = (\d+)\)\)/
name, nlv = $1 == 'name' ? [$2, $3] : [$3, $2]
Expand Down Expand Up @@ -104,7 +104,7 @@
end

specify "should not increment the lock column when the update fails" do
@c.dataset.meta_def(:update) { raise Exception }
@c.instance_dataset.meta_def(:update) { raise Exception }
p1 = @c[1]
p1.modified!
lv = p1.lock_version
Expand Down
4 changes: 4 additions & 0 deletions spec/extensions/prepared_statements_spec.rb
Expand Up @@ -32,6 +32,10 @@
end

specify "should correctly create instance if dataset supports insert_select" do
ds = @c.instance_dataset
def ds.supports_insert_select?
true
end
def @ds.supports_insert_select?
true
end
Expand Down
12 changes: 6 additions & 6 deletions spec/extensions/sharding_spec.rb
Expand Up @@ -5,19 +5,19 @@
@db = Sequel.mock(:numrows=>1, :autoid=>proc{1}, :servers=>{:s1=>{}, :s2=>{}, :s3=>{}, :s4=>{}})
@Artist = Class.new(Sequel::Model(@db[:artists]))
@Artist.class_eval do
dataset._fetch = {:id=>2, :name=>'YJM'}
instance_dataset._fetch = dataset._fetch = {:id=>2, :name=>'YJM'}
columns :id, :name
plugin :sharding
end
@Album = Class.new(Sequel::Model(@db[:albums]))
@Album.class_eval do
dataset._fetch = {:id=>1, :name=>'RF', :artist_id=>2}
instance_dataset._fetch = dataset._fetch = {:id=>1, :name=>'RF', :artist_id=>2}
columns :id, :artist_id, :name
plugin :sharding
end
@Tag = Class.new(Sequel::Model(@db[:tags]))
@Tag.class_eval do
dataset._fetch = {:id=>3, :name=>'M'}
instance_dataset._fetch = dataset._fetch = {:id=>3, :name=>'M'}
columns :id, :name
plugin :sharding
end
Expand Down Expand Up @@ -67,7 +67,7 @@
end

specify "should not use current dataset's shard when eager loading if eagerly loaded dataset has its own shard" do
@Artist.dataset.opts[:server] = :s2
@Artist.instance_dataset.opts[:server] = @Artist.dataset.opts[:server] = :s2
albums = @Album.server(:s1).eager(:artist).all
@db.sqls.should == ["SELECT * FROM albums -- s1", "SELECT * FROM artists WHERE (artists.id IN (2)) -- s2"]
albums.length.should == 1
Expand All @@ -86,7 +86,7 @@
end

specify "should not use current dataset's shard when eager graphing if eagerly graphed dataset has its own shard" do
@Artist.dataset.opts[:server] = :s2
@Artist.instance_dataset.opts[:server] = @Artist.dataset.opts[:server] = :s2
ds = @Album.server(:s1).eager_graph(:artist)
ds._fetch = {:id=>1, :artist_id=>2, :name=>'RF', :artist_id_0=>2, :artist_name=>'YJM'}
albums = ds.all
Expand All @@ -97,7 +97,7 @@
end

specify "should use eagerly graphed dataset shard for eagerly graphed objects even if current dataset does not have a shard" do
@Artist.dataset.opts[:server] = :s2
@Artist.instance_dataset.opts[:server] = @Artist.dataset.opts[:server] = :s2
ds = @Album.eager_graph(:artist)
ds._fetch = {:id=>1, :artist_id=>2, :name=>'RF', :artist_id_0=>2, :artist_name=>'YJM'}
albums = ds.all
Expand Down
2 changes: 1 addition & 1 deletion spec/extensions/skip_create_refresh_spec.rb
Expand Up @@ -5,7 +5,7 @@
c = Class.new(Sequel::Model(:a))
c.columns :id, :x
c.db.reset
c.dataset.meta_def(:insert){|*a| super(*a); 2}
c.instance_dataset.meta_def(:insert){|*a| super(*a); 2}
c.create(:x=>1)
c.db.sqls.should == ['INSERT INTO a (x) VALUES (1)', 'SELECT * FROM a WHERE (id = 2) LIMIT 1']

Expand Down
16 changes: 8 additions & 8 deletions spec/model/associations_spec.rb
Expand Up @@ -653,7 +653,7 @@ def self.to_s; 'Node'; end
it "should add a setter method" do
@c2.one_to_one :attribute, :class => @c1
attrib = @c1.new(:id=>3)
@c1.dataset._fetch = {:id=>3}
@c1.dataset._fetch = @c1.instance_dataset._fetch = {:id=>3}
@c2.new(:id => 1234).attribute = attrib
sqls = MODEL_DB.sqls
['INSERT INTO attributes (node_id, id) VALUES (1234, 3)',
Expand Down Expand Up @@ -692,7 +692,7 @@ def self.to_s; 'Node'; end
it "should have the setter method respect the :primary_key option" do
@c2.one_to_one :attribute, :class => @c1, :primary_key=>:xxx
attrib = @c1.new(:id=>3)
@c1.dataset._fetch = {:id=>3}
@c1.dataset._fetch = @c1.instance_dataset._fetch = {:id=>3}
@c2.new(:id => 1234, :xxx=>5).attribute = attrib
sqls = MODEL_DB.sqls
['INSERT INTO attributes (node_id, id) VALUES (5, 3)',
Expand Down Expand Up @@ -836,7 +836,7 @@ class Parent < Sequel::Model; end

d = @c2.new(:id => 1)
f = @c2.new(:id => 3, :node_id=> 4321)
@c2.dataset._fetch = {:id => 3, :node_id=>1}
@c2.dataset._fetch = @c2.instance_dataset._fetch = {:id => 3, :node_id=>1}
d.parent = f
f.values.should == {:id => 3, :node_id=>1}
d.parent.should == f
Expand All @@ -854,7 +854,7 @@ class Parent < Sequel::Model; end
@c2.one_to_one :parent, :class => @c2, :primary_key=>:blah
d = @c2.new(:id => 1, :blah => 3)
e = @c2.new(:id => 4321, :node_id=>444)
@c2.dataset._fetch = {:id => 4321, :node_id => 3}
@c2.dataset._fetch = @c2.instance_dataset._fetch = {:id => 4321, :node_id => 3}
d.parent = e
e.values.should == {:id => 4321, :node_id => 3}
sqls = MODEL_DB.sqls
Expand All @@ -867,7 +867,7 @@ class Parent < Sequel::Model; end
@c2.one_to_one :parent, :class => @c2, :key=>:blah
d = @c2.new(:id => 3)
e = @c2.new(:id => 4321, :blah=>444)
@c2.dataset._fetch = {:id => 4321, :blah => 3}
@c2.dataset._fetch = @c2.instance_dataset._fetch = {:id => 4321, :blah => 3}
d.parent = e
e.values.should == {:id => 4321, :blah => 3}
sqls = MODEL_DB.sqls
Expand Down Expand Up @@ -1181,7 +1181,7 @@ class Value < Sequel::Model; end

n = @c2.load(:id => 1234)
a = @c1.new(:id => 234)
@c1.dataset._fetch = {:node_id => 1234, :id => 234}
@c1.dataset._fetch = @c1.instance_dataset._fetch = {:node_id => 1234, :id => 234}
a.should == n.add_attribute(a)
sqls = MODEL_DB.sqls
sqls.shift.should =~ /INSERT INTO attributes \((node_)?id, (node_)?id\) VALUES \(1?234, 1?234\)/
Expand Down Expand Up @@ -1213,7 +1213,7 @@ class Value < Sequel::Model; end
@c2.one_to_many :attributes, :class => @c1
n = @c2.new(:id => 1234)
MODEL_DB.reset
@c1.dataset._fetch = {:node_id => 1234, :id => 234}
@c1.dataset._fetch = @c1.instance_dataset._fetch = {:node_id => 1234, :id => 234}
n.add_attribute(:id => 234).should == @c1.load(:node_id => 1234, :id => 234)
sqls = MODEL_DB.sqls
sqls.shift.should =~ /INSERT INTO attributes \((node_)?id, (node_)?id\) VALUES \(1?234, 1?234\)/
Expand Down Expand Up @@ -1969,7 +1969,7 @@ class Tag < Sequel::Model; end
@c2.many_to_many :attributes, :class => @c1

n = @c2.load(:id => 1234)
@c1.dataset._fetch = {:id=>1}
@c1.dataset._fetch = @c1.instance_dataset._fetch = {:id=>1}
n.add_attribute(:id => 1).should == @c1.load(:id => 1)
sqls = MODEL_DB.sqls
['INSERT INTO attributes_nodes (node_id, attribute_id) VALUES (1234, 1)',
Expand Down
8 changes: 4 additions & 4 deletions spec/model/base_spec.rb
Expand Up @@ -407,7 +407,7 @@ class ::Leopard < Feline; end
i.set(:x => 4, :y => 5, :z => 6)
i.values.should == {:x => 4, :y => 5}

@c.dataset._fetch = {:x => 7}
@c.instance_dataset._fetch = @c.dataset._fetch = {:x => 7}
i = @c.new
i.update(:x => 7, :z => 9)
i.values.should == {:x => 7}
Expand All @@ -421,7 +421,7 @@ class ::Leopard < Feline; end
i.set(:x => 4, :y => 5, :z => 6)
i.values.should == {:x => 4, :y => 5}

@c.dataset._fetch = {:x => 7}
@c.instance_dataset._fetch = @c.dataset._fetch = {:x => 7}
i = @c.new
i.update(:x => 7, :z => 9)
i.values.should == {:x => 7}
Expand All @@ -436,7 +436,7 @@ class ::Leopard < Feline; end
i.set(:x => 4, :y => 5, :z => 6)
i.values.should == {:x => 4, :y => 5}

@c.dataset._fetch = {:y => 7}
@c.instance_dataset._fetch = @c.dataset._fetch = {:y => 7}
i = @c.new
i.update(:y => 7, :z => 9)
i.values.should == {:y => 7}
Expand Down Expand Up @@ -610,7 +610,7 @@ class ::Leopard < Feline; end

it "should use Dataset#with_sql if simple_table and simple_pk are true" do
@c.set_dataset :a
@c.dataset._fetch = {:id => 1}
@c.instance_dataset._fetch = @c.dataset._fetch = {:id => 1}
@c[1].should == @c.load(:id=>1)
@db.sqls.should == ['SELECT * FROM "a" WHERE "id" = 1']
end
Expand Down
12 changes: 6 additions & 6 deletions spec/model/model_spec.rb
Expand Up @@ -339,8 +339,8 @@ class ::Album < Sequel::Model(@db[:table.identifier]); end
end

it "should use the last inserted id as primary key if not in values" do
@m.dataset._fetch = {:x => 1, :id => 1234}
@m.dataset.autoid = 1234
@m.instance_dataset._fetch = @m.dataset._fetch = {:x => 1, :id => 1234}
@m.instance_dataset.autoid = @m.dataset.autoid = 1234

o = @m.new(:x => 1)
o.save
Expand Down Expand Up @@ -438,17 +438,17 @@ class ::Album < Sequel::Model(@db[:table.identifier]); end
end

it "should create the record if not found" do
@c.dataset._fetch = [[], {:x=>1, :id=>1}]
@c.dataset.autoid = 1
@c.instance_dataset._fetch = @c.dataset._fetch = [[], {:x=>1, :id=>1}]
@c.instance_dataset.autoid = @c.dataset.autoid = 1
@c.find_or_create(:x => 1).should == @c.load(:x=>1, :id=>1)
MODEL_DB.sqls.should == ["SELECT * FROM items WHERE (x = 1) LIMIT 1",
"INSERT INTO items (x) VALUES (1)",
"SELECT * FROM items WHERE (id = 1) LIMIT 1"]
end

it "should pass the new record to be created to the block if no record is found" do
@c.dataset._fetch = [[], {:x=>1, :id=>1}]
@c.dataset.autoid = 1
@c.instance_dataset._fetch = @c.dataset._fetch = [[], {:x=>1, :id=>1}]
@c.instance_dataset.autoid = @c.dataset.autoid = 1
@c.find_or_create(:x => 1){|x| x[:y] = 2}.should == @c.load(:x=>1, :id=>1)
sqls = MODEL_DB.sqls
sqls.first.should == "SELECT * FROM items WHERE (x = 1) LIMIT 1"
Expand Down

0 comments on commit 8accebb

Please sign in to comment.