From 7f6f91d9f8c0966e24c8178f2df7493559cc1c4f Mon Sep 17 00:00:00 2001 From: "tzik@chromium.org" Date: Thu, 24 Jul 2014 18:27:48 +0000 Subject: [PATCH] [SyncFS] Make the snapshot sync of local-to-remote sync parallelization enabled. Support snapshot-sync on demoted URL. Previous implementation drops changes in demoted changes, and doesn't mirror new changes on demoted URLs. BUG=344769 TEST=unit_tests --gtest_filter="LocalFileChangeTrackerTest.*" Review URL: https://codereview.chromium.org/394033005 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@285335 0039d316-1c4b-4281-b951-d872f2087c98 --- .../local/local_file_change_tracker.cc | 98 ++++++++++++---------- .../local/local_file_change_tracker.h | 7 +- .../local/local_file_change_tracker_unittest.cc | 12 ++- 3 files changed, 67 insertions(+), 50 deletions(-) diff --git a/chrome/browser/sync_file_system/local/local_file_change_tracker.cc b/chrome/browser/sync_file_system/local/local_file_change_tracker.cc index d112285047c9..1adb927c02ce 100644 --- a/chrome/browser/sync_file_system/local/local_file_change_tracker.cc +++ b/chrome/browser/sync_file_system/local/local_file_change_tracker.cc @@ -79,7 +79,7 @@ LocalFileChangeTracker::LocalFileChangeTracker( : initialized_(false), file_task_runner_(file_task_runner), tracker_db_(new TrackerDB(base_path, env_override)), - current_change_seq_(0), + current_change_seq_number_(0), num_changes_(0) { } @@ -90,7 +90,7 @@ LocalFileChangeTracker::~LocalFileChangeTracker() { void LocalFileChangeTracker::OnStartUpdate(const FileSystemURL& url) { DCHECK(file_task_runner_->RunsTasksOnCurrentThread()); - if (ContainsKey(changes_, url)) + if (ContainsKey(changes_, url) || ContainsKey(demoted_changes_, url)) return; // TODO(nhiroki): propagate the error code (see http://crbug.com/152127). MarkDirtyOnDatabase(url); @@ -186,7 +186,7 @@ void LocalFileChangeTracker::RemoveMirrorAndCommitChangesForURL( return; mirror_changes_.erase(found); - if (ContainsKey(changes_, url)) + if (ContainsKey(changes_, url) || ContainsKey(demoted_changes_, url)) MarkDirtyOnDatabase(url); else ClearDirtyOnDatabase(url); @@ -202,29 +202,28 @@ void LocalFileChangeTracker::ResetToMirrorAndCommitChangesForURL( return; } const ChangeInfo& info = found->second; - change_seqs_[info.change_seq] = url; - changes_[url] = info; + if (ContainsKey(demoted_changes_, url)) { + DCHECK(!ContainsKey(changes_, url)); + demoted_changes_[url] = info; + } else { + DCHECK(!ContainsKey(demoted_changes_, url)); + change_seqs_[info.change_seq] = url; + changes_[url] = info; + } RemoveMirrorAndCommitChangesForURL(url); } void LocalFileChangeTracker::DemoteChangesForURL( const fileapi::FileSystemURL& url) { DCHECK(file_task_runner_->RunsTasksOnCurrentThread()); + FileChangeMap::iterator found = changes_.find(url); if (found == changes_.end()) return; - FileChangeList changes = found->second.change_list; - - mirror_changes_.erase(url); + DCHECK(!ContainsKey(demoted_changes_, url)); change_seqs_.erase(found->second.change_seq); + demoted_changes_.insert(*found); changes_.erase(found); - - FileChangeList::List change_list = changes.list(); - while (!change_list.empty()) { - RecordChangeToChangeMaps(url, change_list.front(), 0, - &demoted_changes_, NULL); - change_list.pop_front(); - } UpdateNumChanges(); } @@ -238,15 +237,12 @@ void LocalFileChangeTracker::PromoteDemotedChangesForURL( FileChangeList::List change_list = iter->second.change_list.list(); // Make sure that this URL is in no queues. + DCHECK(!ContainsKey(change_seqs_, iter->second.change_seq)); DCHECK(!ContainsKey(changes_, url)); - DCHECK(!ContainsKey(mirror_changes_, url)); + change_seqs_[iter->second.change_seq] = url; + changes_.insert(*iter); demoted_changes_.erase(iter); - - while (!change_list.empty()) { - RecordChange(url, change_list.front()); - change_list.pop_front(); - } } bool LocalFileChangeTracker::PromoteDemotedChanges() { @@ -281,24 +277,25 @@ void LocalFileChangeTracker::ResetForFileSystem( for (FileChangeMap::iterator iter = changes_.begin(); iter != changes_.end();) { fileapi::FileSystemURL url = iter->first; - if (url.origin() != origin || url.type() != type) { - ++iter; - continue; - } - mirror_changes_.erase(url); - demoted_changes_.erase(url); - change_seqs_.erase(iter->second.change_seq); - changes_.erase(iter++); - - std::string serialized_url; - const bool should_success = - SerializeSyncableFileSystemURL(url, &serialized_url); - if (!should_success) { - NOTREACHED() << "Failed to serialize: " << url.DebugString(); - continue; - } - batch->Delete(serialized_url); + int change_seq = iter->second.change_seq; + // Advance |iter| before calling ResetForURL to avoid the iterator + // invalidation in it. + ++iter; + if (url.origin() == origin && url.type() == type) + ResetForURL(url, change_seq, batch.get()); } + + for (FileChangeMap::iterator iter = demoted_changes_.begin(); + iter != demoted_changes_.end();) { + fileapi::FileSystemURL url = iter->first; + int change_seq = iter->second.change_seq; + // Advance |iter| before calling ResetForURL to avoid the iterator + // invalidation in it. + ++iter; + if (url.origin() == origin && url.type() == type) + ResetForURL(url, change_seq, batch.get()); + } + // Fail to apply batch to database wouldn't have critical effect, they'll be // just marked deleted on next relaunch. tracker_db_->WriteBatch(batch.Pass()); @@ -415,12 +412,13 @@ SyncStatusCode LocalFileChangeTracker::CollectLastDirtyChanges( void LocalFileChangeTracker::RecordChange( const FileSystemURL& url, const FileChange& change) { DCHECK(file_task_runner_->RunsTasksOnCurrentThread()); + int change_seq = current_change_seq_number_++; if (ContainsKey(demoted_changes_, url)) { - RecordChangeToChangeMaps(url, change, 0, &demoted_changes_, NULL); - return; + RecordChangeToChangeMaps(url, change, change_seq, + &demoted_changes_, NULL); + } else { + RecordChangeToChangeMaps(url, change, change_seq, &changes_, &change_seqs_); } - int change_seq = current_change_seq_++; - RecordChangeToChangeMaps(url, change, change_seq, &changes_, &change_seqs_); if (ContainsKey(mirror_changes_, url)) RecordChangeToChangeMaps(url, change, change_seq, &mirror_changes_, NULL); UpdateNumChanges(); @@ -446,6 +444,22 @@ void LocalFileChangeTracker::RecordChangeToChangeMaps( (*change_seqs)[info.change_seq] = url; } +void LocalFileChangeTracker::ResetForURL(const fileapi::FileSystemURL& url, + int change_seq, + leveldb::WriteBatch* batch) { + mirror_changes_.erase(url); + demoted_changes_.erase(url); + change_seqs_.erase(change_seq); + changes_.erase(url); + + std::string serialized_url; + if (!SerializeSyncableFileSystemURL(url, &serialized_url)) { + NOTREACHED() << "Failed to serialize: " << url.DebugString(); + return; + } + batch->Delete(serialized_url); +} + // TrackerDB ------------------------------------------------------------------- LocalFileChangeTracker::TrackerDB::TrackerDB(const base::FilePath& base_path, diff --git a/chrome/browser/sync_file_system/local/local_file_change_tracker.h b/chrome/browser/sync_file_system/local/local_file_change_tracker.h index 34fd457fbd38..104e1e53ae1e 100644 --- a/chrome/browser/sync_file_system/local/local_file_change_tracker.h +++ b/chrome/browser/sync_file_system/local/local_file_change_tracker.h @@ -31,6 +31,7 @@ class FileSystemURL; namespace leveldb { class Env; +class WriteBatch; } namespace sync_file_system { @@ -164,6 +165,10 @@ class LocalFileChangeTracker FileChangeMap* changes, ChangeSeqMap* change_seqs); + void ResetForURL(const fileapi::FileSystemURL& url, + int change_seq, + leveldb::WriteBatch* batch); + bool initialized_; scoped_refptr file_task_runner_; @@ -179,7 +184,7 @@ class LocalFileChangeTracker // Change sequence number. Briefly gives a hint about the order of changes, // but they are updated when a new change comes on the same file (as // well as Drive's changestamps). - int64 current_change_seq_; + int64 current_change_seq_number_; // This can be accessed on any threads (with num_changes_lock_). int64 num_changes_; diff --git a/chrome/browser/sync_file_system/local/local_file_change_tracker_unittest.cc b/chrome/browser/sync_file_system/local/local_file_change_tracker_unittest.cc index c3b9e9aed98b..cada3cbce706 100644 --- a/chrome/browser/sync_file_system/local/local_file_change_tracker_unittest.cc +++ b/chrome/browser/sync_file_system/local/local_file_change_tracker_unittest.cc @@ -236,13 +236,11 @@ TEST_F(LocalFileChangeTrackerTest, GetChanges) { urls_to_process.clear(); change_tracker()->GetNextChangedURLs(&urls_to_process, 0); ASSERT_EQ(5U, urls_to_process.size()); - EXPECT_EQ(URL(kPath2), urls_to_process[0]); - EXPECT_EQ(URL(kPath5), urls_to_process[1]); - EXPECT_EQ(URL(kPath4), urls_to_process[2]); - EXPECT_TRUE(URL(kPath1) == urls_to_process[3] || - URL(kPath1) == urls_to_process[4]); - EXPECT_TRUE(URL(kPath3) == urls_to_process[3] || - URL(kPath3) == urls_to_process[4]); + EXPECT_EQ(URL(kPath1), urls_to_process[0]); + EXPECT_EQ(URL(kPath2), urls_to_process[1]); + EXPECT_EQ(URL(kPath3), urls_to_process[2]); + EXPECT_EQ(URL(kPath5), urls_to_process[3]); + EXPECT_EQ(URL(kPath4), urls_to_process[4]); // No changes to promote any more. EXPECT_FALSE(change_tracker()->PromoteDemotedChanges()); -- 2.11.4.GIT