diff options
author | Yorick Peterse <yorickpeterse@gmail.com> | 2016-06-29 12:20:31 +0000 |
---|---|---|
committer | Robert Speicher <rspeicher@gmail.com> | 2016-06-29 10:55:24 -0400 |
commit | ed06a211dfc20b56654e2c3b27d678805af68b6f (patch) | |
tree | 3f2f09b4d8056383ab069195223c153fe1ca414f | |
parent | f3de17fd24a3932e240cb37797fe330bf7005997 (diff) | |
download | gitlab-ce-ed06a211dfc20b56654e2c3b27d678805af68b6f.tar.gz |
Merge branch '18663-avoid-extra-yaml-serializations' into 'master'
Use update_columns to by_pass all the dirty code on active_record
See merge request !4985
(cherry picked from commit ad09fcb5b95478cbf7075c6add11cf4fc3430db2)
-rw-r--r-- | CHANGELOG | 1 | ||||
-rw-r--r-- | app/models/merge_request_diff.rb | 47 |
2 files changed, 38 insertions, 10 deletions
diff --git a/CHANGELOG b/CHANGELOG index 738af214fc5..264e6890316 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -14,6 +14,7 @@ v 8.9.3 - Removed fade when filtering results. !4932 - Fix missing avatar on system notes. !4954 - Reduce overhead and optimize ProjectTeam#max_member_access performance. !4973 + - Use update_columns to by_pass all the dirty code on active_record. !4985 v 8.9.2 - Fix visibility of snippets when searching. diff --git a/app/models/merge_request_diff.rb b/app/models/merge_request_diff.rb index aca377cc600..86331a33c05 100644 --- a/app/models/merge_request_diff.rb +++ b/app/models/merge_request_diff.rb @@ -108,44 +108,46 @@ class MergeRequestDiff < ActiveRecord::Base # Reload all commits related to current merge request from repo # and save it as array of hashes in st_commits db field def reload_commits + new_attributes = {} + commit_objects = unmerged_commits if commit_objects.present? - self.st_commits = dump_commits(commit_objects) + new_attributes[:st_commits] = dump_commits(commit_objects) end - save + update_columns_serialized(new_attributes) end # Reload diffs between branches related to current merge request from repo # and save it as array of hashes in st_diffs db field def reload_diffs + new_attributes = {} new_diffs = [] if commits.size.zero? - self.state = :empty + new_attributes[:state] = :empty else diff_collection = unmerged_diffs if diff_collection.overflow? # Set our state to 'overflow' to make the #empty? and #collected? # methods (generated by StateMachine) return false. - self.state = :overflow + new_attributes[:state] = :overflow end - self.real_size = diff_collection.real_size + new_attributes[:real_size] = diff_collection.real_size if diff_collection.any? new_diffs = dump_diffs(diff_collection) - self.state = :collected + new_attributes[:state] = :collected end end - self.st_diffs = new_diffs - - self.base_commit_sha = self.repository.merge_base(self.head, self.base) + new_attributes[:st_diffs] = new_diffs + new_attributes[:base_commit_sha] = self.repository.merge_base(self.head, self.base) - self.save + update_columns_serialized(new_attributes) end # Collect array of Git::Diff objects @@ -190,4 +192,29 @@ class MergeRequestDiff < ActiveRecord::Base ) end end + + private + + # + # #save or #update_attributes providing changes on serialized attributes do a lot of + # serialization and deserialization calls resulting in bad performance. + # Using #update_columns solves the problem with just one YAML.dump per serialized attribute that we provide. + # As a tradeoff we need to reload the current instance to properly manage time objects on those serialized + # attributes. So to keep the same behaviour as the attribute assignment we reload the instance. + # The difference is in the usage of + # #write_attribute= (#update_attributes) and #raw_write_attribute= (#update_columns) + # + # Ex: + # + # new_attributes[:st_commits].first.slice(:committed_date) + # => {:committed_date=>2014-02-27 11:01:38 +0200} + # YAML.load(YAML.dump(new_attributes[:st_commits].first.slice(:committed_date))) + # => {:committed_date=>2014-02-27 10:01:38 +0100} + # + def update_columns_serialized(new_attributes) + return unless new_attributes.any? + + update_columns(new_attributes.merge(updated_at: current_time_from_proper_timezone)) + reload + end end |