summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
Diffstat
-rw-r--r--AUTHORS1
-rwxr-xr-xchrome/browser/sync/syncable/syncable.cc27
-rwxr-xr-xchrome/browser/sync/syncable/syncable.h15
-rwxr-xr-xchrome/browser/sync/syncable/syncable_unittest.cc155
4 files changed, 188 insertions, 10 deletions
diff --git a/AUTHORS b/AUTHORS
index ad09cd8..704fc82 100644
--- a/AUTHORS
+++ b/AUTHORS
@@ -63,3 +63,4 @@ Vernon Tang <vt@foilhead.net>
Alexander Sulfrian <alexander@sulfrian.net>
Philippe Beaudoin <philippe.beaudoin@gmail.com>
Mark Hahnenberg <mhahnenb@gmail.com>
+Alex Gartrell <alexgartrell@gmail.com>
diff --git a/chrome/browser/sync/syncable/syncable.cc b/chrome/browser/sync/syncable/syncable.cc
index ff0ab77..b3df77e 100755
--- a/chrome/browser/sync/syncable/syncable.cc
+++ b/chrome/browser/sync/syncable/syncable.cc
@@ -166,6 +166,7 @@ Directory::Kernel::Kernel(const FilePath& db_path,
extended_attributes(new ExtendedAttributes),
unapplied_update_metahandles(new MetahandleSet),
unsynced_metahandles(new MetahandleSet),
+ dirty_metahandles(new MetahandleSet),
channel(new Directory::Channel(syncable::DIRECTORY_DESTROYED)),
changes_channel(new Directory::ChangesChannel(kShutdownChangesEvent)),
last_sync_timestamp_(info.kernel_info.last_sync_timestamp),
@@ -196,6 +197,7 @@ Directory::Kernel::~Kernel() {
delete changes_channel;
delete unsynced_metahandles;
delete unapplied_update_metahandles;
+ delete dirty_metahandles;
delete extended_attributes;
delete parent_id_child_index;
delete client_tag_index;
@@ -232,6 +234,7 @@ void Directory::InitializeIndices() {
kernel_->unsynced_metahandles->insert(entry->ref(META_HANDLE));
if (entry->ref(IS_UNAPPLIED_UPDATE))
kernel_->unapplied_update_metahandles->insert(entry->ref(META_HANDLE));
+ DCHECK(!entry->is_dirty());
}
}
@@ -484,6 +487,16 @@ void Directory::ReindexParentId(EntryKernel* const entry,
CHECK(kernel_->parent_id_child_index->insert(entry).second);
}
+void Directory::AddToDirtyMetahandles(int64 handle) {
+ kernel_->transaction_mutex.AssertAcquired();
+ kernel_->dirty_metahandles->insert(handle);
+}
+
+void Directory::ClearDirtyMetahandles() {
+ kernel_->transaction_mutex.AssertAcquired();
+ kernel_->dirty_metahandles->clear();
+}
+
// static
bool Directory::SafeToPurgeFromMemory(const EntryKernel* const entry) {
return entry->ref(IS_DEL) && !entry->is_dirty() && !entry->ref(SYNCING) &&
@@ -495,14 +508,17 @@ void Directory::TakeSnapshotForSaveChanges(SaveChangesSnapshot* snapshot) {
ScopedKernelLock lock(this);
// Deep copy dirty entries from kernel_->metahandles_index into snapshot and
// clear dirty flags.
- for (MetahandlesIndex::iterator i = kernel_->metahandles_index->begin();
- i != kernel_->metahandles_index->end(); ++i) {
- EntryKernel* entry = *i;
+
+ for (MetahandleSet::const_iterator i = kernel_->dirty_metahandles->begin();
+ i != kernel_->dirty_metahandles->end(); ++i) {
+ EntryKernel* entry = GetEntryByHandle(*i, &lock);
+ // Skip over false positives; it happens relatively infrequently.
if (!entry->is_dirty())
continue;
snapshot->dirty_metas.insert(snapshot->dirty_metas.end(), *entry);
entry->clear_dirty();
}
+ ClearDirtyMetahandles();
// Do the same for extended attributes.
for (ExtendedAttributes::iterator i = kernel_->extended_attributes->begin();
@@ -986,6 +1002,11 @@ WriteTransaction::~WriteTransaction() {
else
directory()->CheckTreeInvariants(this, originals_);
}
+
+ for (OriginalEntries::const_iterator i = originals_->begin();
+ i != originals_->end(); ++i) {
+ directory()->AddToDirtyMetahandles(i->ref(META_HANDLE));
+ }
UnlockAndLog(originals_);
}
diff --git a/chrome/browser/sync/syncable/syncable.h b/chrome/browser/sync/syncable/syncable.h
index fd328b3..1bb529d 100755
--- a/chrome/browser/sync/syncable/syncable.h
+++ b/chrome/browser/sync/syncable/syncable.h
@@ -29,6 +29,7 @@
#include "chrome/browser/sync/util/event_sys.h"
#include "chrome/browser/sync/util/row_iterator.h"
#include "chrome/browser/sync/util/sync_types.h"
+#include "testing/gtest/include/gtest/gtest_prod.h" // For FRIEND_TEST
struct PurgeInfo;
@@ -648,6 +649,10 @@ class Directory {
friend class ScopedKernelLock;
friend class ScopedKernelUnlock;
friend class WriteTransaction;
+ friend class SyncableDirectoryTest;
+ FRIEND_TEST(SyncableDirectoryTest, TakeSnapshotGetsAllDirtyHandlesTest);
+ FRIEND_TEST(SyncableDirectoryTest, TakeSnapshotGetsOnlyDirtyHandlesTest);
+
public:
// Various data that the Directory::Kernel we are backing (persisting data
// for) needs saved across runs of the application.
@@ -736,6 +741,8 @@ class Directory {
EntryKernel* GetRootEntry();
bool ReindexId(EntryKernel* const entry, const Id& new_id);
void ReindexParentId(EntryKernel* const entry, const Id& new_parent_id);
+ void AddToDirtyMetahandles(int64 handle);
+ void ClearDirtyMetahandles();
// These don't do semantic checking.
// The semantic checking is implemented higher up.
@@ -930,12 +937,12 @@ class Directory {
EntryKernel needle;
ExtendedAttributes* const extended_attributes;
- // 2 in-memory indices on bits used extremely frequently by the syncer.
+ // 3 in-memory indices on bits used extremely frequently by the syncer.
MetahandleSet* const unapplied_update_metahandles;
MetahandleSet* const unsynced_metahandles;
- // TODO(timsteele): Add a dirty_metahandles index as we now may want to
- // optimize the SaveChanges work of scanning all entries to find dirty ones
- // due to the entire entry domain now being in-memory.
+ // Contains metahandles that are most likely dirty (though not
+ // necessarily). Dirtyness is confirmed in TakeSnapshotForSaveChanges().
+ MetahandleSet* const dirty_metahandles;
// TODO(ncarter): Figure out what the hell this is, and comment it.
Channel* const channel;
diff --git a/chrome/browser/sync/syncable/syncable_unittest.cc b/chrome/browser/sync/syncable/syncable_unittest.cc
index d94815a..3d514e0e 100755
--- a/chrome/browser/sync/syncable/syncable_unittest.cc
+++ b/chrome/browser/sync/syncable/syncable_unittest.cc
@@ -243,9 +243,6 @@ TEST_F(SyncableGeneralTest, ClientIndexRebuildsDeletedProperly) {
}
}
-
-namespace {
-
// A Directory whose backing store always fails SaveChanges by returning false.
class TestUnsaveableDirectory : public Directory {
public:
@@ -316,11 +313,159 @@ class SyncableDirectoryTest : public testing::Test {
bool set_server_fields, bool is_dir, bool add_to_lru, int64 *meta_handle);
};
+TEST_F(SyncableDirectoryTest, TakeSnapshotGetsAllDirtyHandlesTest) {
+ const int metahandles_to_create = 100;
+ std::vector<int64> expected_dirty_metahandles;
+ {
+ WriteTransaction trans(dir_.get(), UNITTEST, __FILE__, __LINE__);
+ for (int i = 0; i < metahandles_to_create; i++) {
+ MutableEntry e(&trans, CREATE, trans.root_id(), "foo");
+ expected_dirty_metahandles.push_back(e.Get(META_HANDLE));
+ e.Put(IS_UNSYNCED, true);
+ }
+ }
+ // Fake SaveChanges() and make sure we got what we expected.
+ {
+ Directory::SaveChangesSnapshot snapshot;
+ AutoLock scoped_lock(dir_->kernel_->save_changes_mutex);
+ dir_->TakeSnapshotForSaveChanges(&snapshot);
+ // Make sure there's an entry for each new metahandle. Make sure all
+ // entries are marked dirty.
+ ASSERT_EQ(expected_dirty_metahandles.size(), snapshot.dirty_metas.size());
+ for (OriginalEntries::const_iterator i = snapshot.dirty_metas.begin();
+ i != snapshot.dirty_metas.end(); ++i) {
+ ASSERT_TRUE(i->is_dirty());
+ }
+ dir_->VacuumAfterSaveChanges(snapshot);
+ }
+ // Put a new value with existing transactions as well as adding new ones.
+ {
+ WriteTransaction trans(dir_.get(), UNITTEST, __FILE__, __LINE__);
+ std::vector<int64> new_dirty_metahandles;
+ for (std::vector<int64>::const_iterator i =
+ expected_dirty_metahandles.begin();
+ i != expected_dirty_metahandles.end(); ++i) {
+ // Change existing entries to directories to dirty them.
+ MutableEntry e1(&trans, GET_BY_HANDLE, *i);
+ e1.Put(IS_DIR, true);
+ e1.Put(IS_UNSYNCED, true);
+ // Add new entries
+ MutableEntry e2(&trans, CREATE, trans.root_id(), "bar");
+ e2.Put(IS_UNSYNCED, true);
+ new_dirty_metahandles.push_back(e2.Get(META_HANDLE));
+ }
+ expected_dirty_metahandles.insert(expected_dirty_metahandles.end(),
+ new_dirty_metahandles.begin(), new_dirty_metahandles.end());
+ }
+ // Fake SaveChanges() and make sure we got what we expected.
+ {
+ Directory::SaveChangesSnapshot snapshot;
+ AutoLock scoped_lock(dir_->kernel_->save_changes_mutex);
+ dir_->TakeSnapshotForSaveChanges(&snapshot);
+ // Make sure there's an entry for each new metahandle. Make sure all
+ // entries are marked dirty.
+ EXPECT_EQ(expected_dirty_metahandles.size(), snapshot.dirty_metas.size());
+ for (OriginalEntries::const_iterator i = snapshot.dirty_metas.begin();
+ i != snapshot.dirty_metas.end(); ++i) {
+ EXPECT_TRUE(i->is_dirty());
+ }
+ dir_->VacuumAfterSaveChanges(snapshot);
+ }
+}
+
+TEST_F(SyncableDirectoryTest, TakeSnapshotGetsOnlyDirtyHandlesTest) {
+ const int metahandles_to_create = 100;
+ const unsigned int number_changed = 100u; // half of 2 * metahandles_to_create
+ std::vector<int64> expected_dirty_metahandles;
+ {
+ WriteTransaction trans(dir_.get(), UNITTEST, __FILE__, __LINE__);
+ for (int i = 0; i < metahandles_to_create; i++) {
+ MutableEntry e(&trans, CREATE, trans.root_id(), "foo");
+ expected_dirty_metahandles.push_back(e.Get(META_HANDLE));
+ e.Put(IS_UNSYNCED, true);
+ }
+ }
+ dir_->SaveChanges();
+ // Put a new value with existing transactions as well as adding new ones.
+ {
+ WriteTransaction trans(dir_.get(), UNITTEST, __FILE__, __LINE__);
+ std::vector<int64> new_dirty_metahandles;
+ for (std::vector<int64>::const_iterator i =
+ expected_dirty_metahandles.begin();
+ i != expected_dirty_metahandles.end(); ++i) {
+ // Change existing entries to directories to dirty them.
+ MutableEntry e1(&trans, GET_BY_HANDLE, *i);
+ ASSERT_TRUE(e1.good());
+ e1.Put(IS_DIR, true);
+ e1.Put(IS_UNSYNCED, true);
+ // Add new entries
+ MutableEntry e2(&trans, CREATE, trans.root_id(), "bar");
+ e2.Put(IS_UNSYNCED, true);
+ new_dirty_metahandles.push_back(e2.Get(META_HANDLE));
+ }
+ expected_dirty_metahandles.insert(expected_dirty_metahandles.end(),
+ new_dirty_metahandles.begin(), new_dirty_metahandles.end());
+ }
+ dir_->SaveChanges();
+ // Don't make any changes whatsoever and ensure nothing comes back.
+ {
+ WriteTransaction trans(dir_.get(), UNITTEST, __FILE__, __LINE__);
+ for (std::vector<int64>::const_iterator i =
+ expected_dirty_metahandles.begin();
+ i != expected_dirty_metahandles.end(); ++i) {
+ MutableEntry e(&trans, GET_BY_HANDLE, *i);
+ ASSERT_TRUE(e.good());
+ // We aren't doing anything to dirty these entries.
+ }
+ }
+ // Fake SaveChanges() and make sure we got what we expected.
+ {
+ Directory::SaveChangesSnapshot snapshot;
+ AutoLock scoped_lock(dir_->kernel_->save_changes_mutex);
+ dir_->TakeSnapshotForSaveChanges(&snapshot);
+ // Make sure there are no dirty_metahandles.
+ EXPECT_EQ(0u, snapshot.dirty_metas.size());
+ dir_->VacuumAfterSaveChanges(snapshot);
+ }
+ {
+ WriteTransaction trans(dir_.get(), UNITTEST, __FILE__, __LINE__);
+ bool should_change = false;
+ for (std::vector<int64>::const_iterator i =
+ expected_dirty_metahandles.begin();
+ i != expected_dirty_metahandles.end(); ++i) {
+ // Maybe change entries by flipping IS_DIR.
+ MutableEntry e(&trans, GET_BY_HANDLE, *i);
+ ASSERT_TRUE(e.good());
+ should_change = !should_change;
+ if (should_change) {
+ bool not_dir = !e.Get(IS_DIR);
+ e.Put(IS_DIR, not_dir);
+ e.Put(IS_UNSYNCED, true);
+ }
+ }
+ }
+ // Fake SaveChanges() and make sure we got what we expected.
+ {
+ Directory::SaveChangesSnapshot snapshot;
+ AutoLock scoped_lock(dir_->kernel_->save_changes_mutex);
+ dir_->TakeSnapshotForSaveChanges(&snapshot);
+ // Make sure there's an entry for each changed metahandle. Make sure all
+ // entries are marked dirty.
+ EXPECT_EQ(number_changed, snapshot.dirty_metas.size());
+ for (OriginalEntries::const_iterator i = snapshot.dirty_metas.begin();
+ i != snapshot.dirty_metas.end(); ++i) {
+ EXPECT_TRUE(i->is_dirty());
+ }
+ dir_->VacuumAfterSaveChanges(snapshot);
+ }
+}
+
const FilePath::CharType SyncableDirectoryTest::kFilePath[] =
FILE_PATH_LITERAL("Test.sqlite3");
const char SyncableDirectoryTest::kName[] = "Foo";
const Id SyncableDirectoryTest::kId(TestIdFactory::FromNumber(-99));
+namespace {
TEST_F(SyncableDirectoryTest, TestBasicLookupNonExistantID) {
ReadTransaction rtrans(dir_.get(), __FILE__, __LINE__);
Entry e(&rtrans, GET_BY_ID, kId);
@@ -942,6 +1087,8 @@ TEST_F(SyncableDirectoryTest, GetModelType) {
}
}
+} // namespace
+
void SyncableDirectoryTest::ValidateEntry(BaseTransaction* trans, int64 id,
bool check_name, string name, int64 base_version, int64 server_version,
bool is_del) {
@@ -955,6 +1102,8 @@ void SyncableDirectoryTest::ValidateEntry(BaseTransaction* trans, int64 id,
ASSERT_TRUE(is_del == e.Get(IS_DEL));
}
+namespace {
+
TEST(SyncableDirectoryManager, TestFileRelease) {
DirectoryManager dm(FilePath(FILE_PATH_LITERAL(".")));
ASSERT_TRUE(dm.Open("ScopeTest"));
generated by cgit v1.1 at 2025-01-16 06:14:08 +0000