diff options
author | Paco Guzman <pacoguzmanp@gmail.com> | 2016-06-24 18:30:33 +0200 |
---|---|---|
committer | Paco Guzman <pacoguzmanp@gmail.com> | 2016-06-29 13:13:54 +0200 |
commit | f525b535a531aeb5c8e5d0771e8a8603e7936e6c (patch) | |
tree | 02b1c12a65ae328a2cec7869c052830bc9d39dde | |
parent | 3611ee5663a25bc619b7c2201908af99dc58debf (diff) | |
download | gitlab-ce-f525b535a531aeb5c8e5d0771e8a8603e7936e6c.tar.gz |
Use update_columns to by_pass all the dirty code on active_record
We think this is valid because we’re just storing new data that always will be valid
-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 5c735bd1420..2840b7aa822 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -21,6 +21,7 @@ v 8.10.0 (unreleased) - Add basic system information like memory and disk usage to the admin panel v 8.9.3 (unreleased) + - MergeRequestDiff reload content use update_columns to avoid multiple YAML de/serializations - Decreased min width of screen to 1280px for pinned sidebar - Fix encrypted data backwards compatibility after upgrading attr_encrypted gem - Update mobile button icons to be more inline with typical UI paradigms 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 |