[Sync] Speed up sync node browser/search in about:sync

chrome/browser/sync/syncable/syncable.cc

 // Copyright (c) 2011 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/sync/syncable/syncable.h"
 
 #include "build/build_config.h"
 
 #include <sys/stat.h>
 #if defined(OS_POSIX)
@@ skipping 488 matching lines @@
   kernel_->needle.put(META_HANDLE, metahandle);
   MetahandlesIndex::iterator found =
       kernel_->metahandles_index->find(&kernel_->needle);
   if (found != kernel_->metahandles_index->end()) {
     // Found it in memory.  Easy.
     return *found;
   }
   return NULL;
 }
 
-void Directory::GetChildHandles(BaseTransaction* trans, const Id& parent_id,
-                                Directory::ChildHandles* result) {
+void Directory::GetChildHandlesById(
+    BaseTransaction* trans, const Id& parent_id,
+    Directory::ChildHandles* result) {
   CHECK(this == trans->directory());
   result->clear();

[lipalani1, 2011/06/02 18:51:52]

Can we move result->clear to the private function?

[akalin, 2011/06/02 22:13:33]

I don't think we should; the other function (GetChildHandlesByHandle needs to
clear it itself because it can return before calling GetChildHandles.

Renamed GetChildHandles to AppendChildHandles to make it more obvious.

   {
     ScopedKernelLock lock(this);

[lipalani1, 2011/06/02 18:51:52]

If the result->clear is moved is the block necessary.

[akalin, 2011/06/02 22:13:33]

The block isn't necessary anyway, so removed it.

-
-    typedef ParentIdChildIndex::iterator iterator;
-    for (iterator i = GetParentChildIndexLowerBound(lock, parent_id),
-                end = GetParentChildIndexUpperBound(lock, parent_id);
-         i != end; ++i) {
-      DCHECK_EQ(parent_id, (*i)->ref(PARENT_ID));
-      result->push_back((*i)->ref(META_HANDLE));
-    }
+    GetChildHandles(lock, parent_id, result);
   }
 }
 
+void Directory::GetChildHandlesByHandle(
+    BaseTransaction* trans, int64 handle,
+    Directory::ChildHandles* result) {
+  CHECK(this == trans->directory());
+  result->clear();
+  {
+    ScopedKernelLock lock(this);
+    EntryKernel* kernel = GetEntryByHandle(handle, &lock);
+    if (!kernel)
+      return;
+
+    GetChildHandles(lock, kernel->ref(ID), result);
+  }
+}
+
 EntryKernel* Directory::GetRootEntry() {
   return GetEntryById(Id());
 }
 
 void ZeroFields(EntryKernel* entry, int first_field) {
   int i = first_field;
   // Note that bitset<> constructor sets all bits to zero, and strings
   // initialize to empty.
   for ( ; i < INT64_FIELDS_END; ++i)
     entry->put(static_cast<Int64Field>(i), 0);
@@ skipping 1409 matching lines @@
 
 Directory::ParentIdChildIndex::iterator
 Directory::GetParentChildIndexUpperBound(const ScopedKernelLock& lock,
                                          const Id& parent_id) {
   // The upper bound of |parent_id|'s range is the lower
   // bound of |++parent_id|'s range.
   return GetParentChildIndexLowerBound(lock,
       parent_id.GetLexicographicSuccessor());
 }
 
+void Directory::GetChildHandles(const ScopedKernelLock& lock,
+                                const Id& parent_id,

[lipalani1, 2011/06/02 18:51:52]

Can this function be const? Also the callers? I think the lock could be a
problem...

[akalin, 2011/06/02 22:13:33]

In this case, it isn't the lock, but the fact that the function it calls
modifies kernel_->needle, which is used as a scratch variable.  Ugh.

+                                Directory::ChildHandles* result) {

[lipalani1, 2011/06/02 18:51:52]

Nit - CHECK(result != NULL)

[akalin, 2011/06/02 22:13:33]

Done.

+  typedef ParentIdChildIndex::iterator iterator;
+  for (iterator i = GetParentChildIndexLowerBound(lock, parent_id),
+           end = GetParentChildIndexUpperBound(lock, parent_id);
+       i != end; ++i) {
+    DCHECK_EQ(parent_id, (*i)->ref(PARENT_ID));
+    result->push_back((*i)->ref(META_HANDLE));
+  }
+}
+
 }  // namespace syncable