From d108288c2f684233298f97f18ac00de0b016deaa Mon Sep 17 00:00:00 2001 From: Dillon Welch Date: Thu, 29 Mar 2018 19:29:55 -0700 Subject: Turn on performance based cops Use attr_reader/attr_writer instead of methods method is 12% slower Use flat_map over map.flatten(1) flatten is 66% slower Use hash[]= instead of hash.merge! with single arguments merge! is 166% slower See https://github.com/rails/rails/pull/32337 for more conversation --- .../connection_specification.rb | 4 +--- .../connection_adapters/mysql/schema_statements.rb | 2 +- .../connection_adapters/mysql2_adapter.rb | 2 +- .../active_record/tasks/mysql_database_tasks.rb | 4 +--- .../tasks/postgresql_database_tasks.rb | 4 +--- .../active_record/tasks/sqlite_database_tasks.rb | 8 +------- activerecord/test/cases/relation_test.rb | 2 +- .../test/cases/tasks/database_tasks_test.rb | 24 +++++++++++----------- 8 files changed, 19 insertions(+), 31 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/connection_adapters/connection_specification.rb b/activerecord/lib/active_record/connection_adapters/connection_specification.rb index 901717ae3d..204691006c 100644 --- a/activerecord/lib/active_record/connection_adapters/connection_specification.rb +++ b/activerecord/lib/active_record/connection_adapters/connection_specification.rb @@ -57,9 +57,7 @@ module ActiveRecord private - def uri - @uri - end + attr_reader :uri def uri_parser @uri_parser ||= URI::Parser.new diff --git a/activerecord/lib/active_record/connection_adapters/mysql/schema_statements.rb b/activerecord/lib/active_record/connection_adapters/mysql/schema_statements.rb index 2087938d7c..1cf210d85b 100644 --- a/activerecord/lib/active_record/connection_adapters/mysql/schema_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/mysql/schema_statements.rb @@ -36,7 +36,7 @@ module ActiveRecord end indexes.last[-2] << row[:Column_name] - indexes.last[-1][:lengths].merge!(row[:Column_name] => row[:Sub_part].to_i) if row[:Sub_part] + indexes.last[-1][:lengths][row[:Column_name]] = row[:Sub_part].to_i if row[:Sub_part] indexes.last[-1][:orders].merge!(row[:Column_name] => :desc) if row[:Collation] == "D" end end diff --git a/activerecord/lib/active_record/connection_adapters/mysql2_adapter.rb b/activerecord/lib/active_record/connection_adapters/mysql2_adapter.rb index 4c57bd48ab..544d720428 100644 --- a/activerecord/lib/active_record/connection_adapters/mysql2_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/mysql2_adapter.rb @@ -117,7 +117,7 @@ module ActiveRecord end def configure_connection - @connection.query_options.merge!(as: :array) + @connection.query_options[:as] = :array super end diff --git a/activerecord/lib/active_record/tasks/mysql_database_tasks.rb b/activerecord/lib/active_record/tasks/mysql_database_tasks.rb index e697fa6def..eddc6fa223 100644 --- a/activerecord/lib/active_record/tasks/mysql_database_tasks.rb +++ b/activerecord/lib/active_record/tasks/mysql_database_tasks.rb @@ -68,9 +68,7 @@ module ActiveRecord private - def configuration - @configuration - end + attr_reader :configuration def configuration_without_database configuration.merge("database" => nil) diff --git a/activerecord/lib/active_record/tasks/postgresql_database_tasks.rb b/activerecord/lib/active_record/tasks/postgresql_database_tasks.rb index 647e066137..533e6953a4 100644 --- a/activerecord/lib/active_record/tasks/postgresql_database_tasks.rb +++ b/activerecord/lib/active_record/tasks/postgresql_database_tasks.rb @@ -90,9 +90,7 @@ module ActiveRecord private - def configuration - @configuration - end + attr_reader :configuration def encoding configuration["encoding"] || DEFAULT_ENCODING diff --git a/activerecord/lib/active_record/tasks/sqlite_database_tasks.rb b/activerecord/lib/active_record/tasks/sqlite_database_tasks.rb index dfe599c4dd..d0bad05176 100644 --- a/activerecord/lib/active_record/tasks/sqlite_database_tasks.rb +++ b/activerecord/lib/active_record/tasks/sqlite_database_tasks.rb @@ -60,13 +60,7 @@ module ActiveRecord private - def configuration - @configuration - end - - def root - @root - end + attr_reader :configuration, :root def run_cmd(cmd, args, out) fail run_cmd_error(cmd, args) unless Kernel.system(cmd, *args, out: out) diff --git a/activerecord/test/cases/relation_test.rb b/activerecord/test/cases/relation_test.rb index fbeb617b29..3ca15640fd 100644 --- a/activerecord/test/cases/relation_test.rb +++ b/activerecord/test/cases/relation_test.rb @@ -187,7 +187,7 @@ module ActiveRecord end relation = Relation.new(klass) - relation.merge!(where: ["foo = ?", "bar"]) + relation.merge!(where: ["foo = ?", "bar"]) # rubocop:disable Performance/RedundantMerge assert_equal Relation::WhereClause.new(["foo = bar"]), relation.where_clause end diff --git a/activerecord/test/cases/tasks/database_tasks_test.rb b/activerecord/test/cases/tasks/database_tasks_test.rb index b9cc08c446..a6efe3fa5e 100644 --- a/activerecord/test/cases/tasks/database_tasks_test.rb +++ b/activerecord/test/cases/tasks/database_tasks_test.rb @@ -152,7 +152,7 @@ module ActiveRecord end def test_ignores_configurations_without_databases - @configurations["development"].merge!("database" => nil) + @configurations["development"]["database"] = nil with_stubbed_configurations_establish_connection do assert_not_called(ActiveRecord::Tasks::DatabaseTasks, :create) do @@ -162,7 +162,7 @@ module ActiveRecord end def test_ignores_remote_databases - @configurations["development"].merge!("host" => "my.server.tld") + @configurations["development"]["host"] = "my.server.tld" with_stubbed_configurations_establish_connection do assert_not_called(ActiveRecord::Tasks::DatabaseTasks, :create) do @@ -172,7 +172,7 @@ module ActiveRecord end def test_warning_for_remote_databases - @configurations["development"].merge!("host" => "my.server.tld") + @configurations["development"]["host"] = "my.server.tld" with_stubbed_configurations_establish_connection do ActiveRecord::Tasks::DatabaseTasks.create_all @@ -183,7 +183,7 @@ module ActiveRecord end def test_creates_configurations_with_local_ip - @configurations["development"].merge!("host" => "127.0.0.1") + @configurations["development"]["host"] = "127.0.0.1" with_stubbed_configurations_establish_connection do assert_called(ActiveRecord::Tasks::DatabaseTasks, :create) do @@ -193,7 +193,7 @@ module ActiveRecord end def test_creates_configurations_with_local_host - @configurations["development"].merge!("host" => "localhost") + @configurations["development"]["host"] = "localhost" with_stubbed_configurations_establish_connection do assert_called(ActiveRecord::Tasks::DatabaseTasks, :create) do @@ -203,7 +203,7 @@ module ActiveRecord end def test_creates_configurations_with_blank_hosts - @configurations["development"].merge!("host" => nil) + @configurations["development"]["host"] = nil with_stubbed_configurations_establish_connection do assert_called(ActiveRecord::Tasks::DatabaseTasks, :create) do @@ -463,7 +463,7 @@ module ActiveRecord end def test_ignores_configurations_without_databases - @configurations[:development].merge!("database" => nil) + @configurations[:development]["database"] = nil ActiveRecord::Base.stub(:configurations, @configurations) do assert_not_called(ActiveRecord::Tasks::DatabaseTasks, :drop) do @@ -473,7 +473,7 @@ module ActiveRecord end def test_ignores_remote_databases - @configurations[:development].merge!("host" => "my.server.tld") + @configurations[:development]["host"] = "my.server.tld" ActiveRecord::Base.stub(:configurations, @configurations) do assert_not_called(ActiveRecord::Tasks::DatabaseTasks, :drop) do @@ -483,7 +483,7 @@ module ActiveRecord end def test_warning_for_remote_databases - @configurations[:development].merge!("host" => "my.server.tld") + @configurations[:development]["host"] = "my.server.tld" ActiveRecord::Base.stub(:configurations, @configurations) do ActiveRecord::Tasks::DatabaseTasks.drop_all @@ -494,7 +494,7 @@ module ActiveRecord end def test_drops_configurations_with_local_ip - @configurations[:development].merge!("host" => "127.0.0.1") + @configurations[:development]["host"] = "127.0.0.1" ActiveRecord::Base.stub(:configurations, @configurations) do assert_called(ActiveRecord::Tasks::DatabaseTasks, :drop) do @@ -504,7 +504,7 @@ module ActiveRecord end def test_drops_configurations_with_local_host - @configurations[:development].merge!("host" => "localhost") + @configurations[:development]["host"] = "localhost" ActiveRecord::Base.stub(:configurations, @configurations) do assert_called(ActiveRecord::Tasks::DatabaseTasks, :drop) do @@ -514,7 +514,7 @@ module ActiveRecord end def test_drops_configurations_with_blank_hosts - @configurations[:development].merge!("host" => nil) + @configurations[:development]["host"] = nil ActiveRecord::Base.stub(:configurations, @configurations) do assert_called(ActiveRecord::Tasks::DatabaseTasks, :drop) do -- cgit v1.2.3