From bcda3477194adb017f131ab8be7888879f64bf91 Mon Sep 17 00:00:00 2001 From: "rlarocque@chromium.org" Date: Thu, 23 May 2013 22:33:09 +0000 Subject: [PATCH] sync: Use a specialized transaction version setter The setting of the transaction version used to be implemented with Put(TRANSACTION_VERSION, ...), like other int64 EntryKernel fields. This had the side effect of calling SaveOriginal() on the entry. That had the side effect of creating unnecessary copies of the EntryKernels. This change creates a new function, MutableEntry::UpdateTransactionVersion() which does not try to save the original entry. It also prevents the use of Put(TRANSACTION_VERSION) and updates the only call site for this function. BUG=241940,241813,238621 Review URL: https://chromiumcodereview.appspot.com/15376002 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@201905 0039d316-1c4b-4281-b951-d872f2087c98 --- sync/syncable/mutable_entry.cc | 10 ++++++++++ sync/syncable/mutable_entry.h | 6 ++++++ sync/syncable/syncable_write_transaction.cc | 2 +- 3 files changed, 17 insertions(+), 1 deletion(-) diff --git a/sync/syncable/mutable_entry.cc b/sync/syncable/mutable_entry.cc index 7c91135ffe07..9698ab7eccfb 100644 --- a/sync/syncable/mutable_entry.cc +++ b/sync/syncable/mutable_entry.cc @@ -157,6 +157,10 @@ bool MutableEntry::PutIsDel(bool is_del) { bool MutableEntry::Put(Int64Field field, const int64& value) { DCHECK(kernel_); + + // We shouldn't set TRANSACTION_VERSION here. See UpdateTransactionVersion. + DCHECK_NE(TRANSACTION_VERSION, field); + write_transaction_->SaveOriginal(kernel_); if (kernel_->ref(field) != value) { ScopedKernelLock lock(dir()); @@ -413,6 +417,12 @@ bool MutableEntry::Put(BitTemp field, bool value) { return true; } +void MutableEntry::UpdateTransactionVersion(int64 value) { + ScopedKernelLock lock(dir()); + kernel_->put(TRANSACTION_VERSION, value); + kernel_->mark_dirty(dir()->kernel_->dirty_metahandles); +} + // This function sets only the flags needed to get this entry to sync. bool MarkForSyncing(MutableEntry* e) { DCHECK_NE(static_cast(NULL), e); diff --git a/sync/syncable/mutable_entry.h b/sync/syncable/mutable_entry.h index 134e4a9b8f24..60c2715e3cf2 100644 --- a/sync/syncable/mutable_entry.h +++ b/sync/syncable/mutable_entry.h @@ -84,6 +84,12 @@ class SYNC_EXPORT_PRIVATE MutableEntry : public Entry { bool Put(BitTemp field, bool value); + // This is similar to what one would expect from Put(TRANSACTION_VERSION), + // except that it doesn't bother to invoke 'SaveOriginals'. Calling that + // function is at best unnecessary, since the transaction will have already + // used its list of mutations by the time this function is called. + void UpdateTransactionVersion(int64 version); + protected: syncable::MetahandleSet* GetDirtyIndexHelper(); diff --git a/sync/syncable/syncable_write_transaction.cc b/sync/syncable/syncable_write_transaction.cc index 478792634091..04113a74114b 100644 --- a/sync/syncable/syncable_write_transaction.cc +++ b/sync/syncable/syncable_write_transaction.cc @@ -134,7 +134,7 @@ void WriteTransaction::UpdateTransactionVersion( directory_->IncrementTransactionVersion(type); type_seen.Put(type); } - entry.Put(TRANSACTION_VERSION, directory_->GetTransactionVersion(type)); + entry.UpdateTransactionVersion(directory_->GetTransactionVersion(type)); } }