Skip to content

Commit

Permalink
sync: Count nodes more efficiently
Browse files Browse the repository at this point in the history
The GetTotalNodesCount() function is called sufficiently often that it
makes sense to spend some effort optimizing its implementation.

The current implementation uses some traversal functions that are fairly
slow, especially since GetSuccessorId() was made more expensive by the
implementation of UniquePositions.

This change moves the implementation into the syncable::Directory.  This
allows it to take advantage of the Directory's internal data structures
and avoid unnecessary lookups. 

BUG=248143,238621

Review URL: https://chromiumcodereview.appspot.com/15322003

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@202973 0039d316-1c4b-4281-b951-d872f2087c98
  • Loading branch information
rlarocque@chromium.org committed May 29, 2013
1 parent f7435c2 commit bb7e03b
Show file tree
Hide file tree
Showing 5 changed files with 55 additions and 25 deletions.
26 changes: 1 addition & 25 deletions sync/internal_api/base_node.cc
Original file line number Diff line number Diff line change
Expand Up @@ -220,31 +220,7 @@ void BaseNode::GetChildIds(std::vector<int64>* result) const {
}

int BaseNode::GetTotalNodeCount() const {
syncable::BaseTransaction* trans = GetTransaction()->GetWrappedTrans();

int count = 1; // Start with one to include the node itself.

std::stack<int64> stack;
stack.push(GetFirstChildId());
while (!stack.empty()) {
int64 handle = stack.top();
stack.pop();
if (handle == kInvalidId)
continue;
count++;
syncable::Entry entry(trans, syncable::GET_BY_HANDLE, handle);
if (!entry.good())
continue;
syncable::Id successor_id = entry.GetSuccessorId();
if (!successor_id.IsRoot())
stack.push(IdToMetahandle(trans, successor_id));
if (!entry.Get(syncable::IS_DIR))
continue;
syncable::Id child_id = entry.GetFirstChildId();
if (!child_id.IsRoot())
stack.push(IdToMetahandle(trans, child_id));
}
return count;
return GetEntry()->GetTotalNodeCount();
}

DictionaryValue* BaseNode::GetSummaryAsValue() const {
Expand Down
40 changes: 40 additions & 0 deletions sync/syncable/directory.cc
Original file line number Diff line number Diff line change
Expand Up @@ -321,6 +321,46 @@ bool Directory::GetChildHandlesByHandle(
return true;
}

int Directory::GetTotalNodeCount(
BaseTransaction* trans,
EntryKernel* kernel) const {
if (!SyncAssert(this == trans->directory(), FROM_HERE,
"Directories don't match", trans))
return false;

int count = 1;
std::deque<const OrderedChildSet*> child_sets;

GetChildSetForKernel(trans, kernel, &child_sets);
while (!child_sets.empty()) {
const OrderedChildSet* set = child_sets.front();
child_sets.pop_front();
for (OrderedChildSet::const_iterator it = set->begin();
it != set->end(); ++it) {
count++;
GetChildSetForKernel(trans, *it, &child_sets);
}
}

return count;
}

void Directory::GetChildSetForKernel(
BaseTransaction* trans,
EntryKernel* kernel,
std::deque<const OrderedChildSet*>* child_sets) const {
if (!kernel->ref(IS_DIR))
return; // Not a directory => no children.

const OrderedChildSet* descendants =
kernel_->parent_child_index->GetChildren(kernel->ref(ID));
if (!descendants)
return; // This directory has no children.

// Add our children to the list of items to be traversed.
child_sets->push_back(descendants);
}

EntryKernel* Directory::GetRootEntry() {
return GetEntryById(Id());
}
Expand Down
9 changes: 9 additions & 0 deletions sync/syncable/directory.h
Original file line number Diff line number Diff line change
Expand Up @@ -321,6 +321,9 @@ class SYNC_EXPORT Directory {
bool GetChildHandlesByHandle(BaseTransaction*, int64 handle,
ChildHandles* result);

// Counts all items under the given node, including the node itself.
int GetTotalNodeCount(BaseTransaction*, EntryKernel* kernel_) const;

// Returns true iff |id| has children.
bool HasChildren(BaseTransaction* trans, const Id& id);

Expand Down Expand Up @@ -454,6 +457,12 @@ class SYNC_EXPORT Directory {
bool SafeToPurgeFromMemory(WriteTransaction* trans,
const EntryKernel* const entry) const;

// A helper used by GetTotalNodeCount.
void GetChildSetForKernel(
BaseTransaction*,
EntryKernel* kernel_,
std::deque<const OrderedChildSet*>* child_sets) const;

Directory& operator = (const Directory&);

public:
Expand Down
4 changes: 4 additions & 0 deletions sync/syncable/entry.cc
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,10 @@ void Entry::GetChildHandles(std::vector<int64>* result) const {
dir()->GetChildHandlesById(basetrans_, Get(ID), result);
}

int Entry::GetTotalNodeCount() const {
return dir()->GetTotalNodeCount(basetrans_, kernel_);
}

bool Entry::ShouldMaintainPosition() const {
return kernel_->ShouldMaintainPosition();
}
Expand Down
1 change: 1 addition & 0 deletions sync/syncable/entry.h
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,7 @@ class SYNC_EXPORT Entry {
Id GetPredecessorId() const;
Id GetSuccessorId() const;
Id GetFirstChildId() const;
int GetTotalNodeCount() const;

// Returns a vector of this node's children's handles.
// Clears |result| if there are no children. If this node is of a type that
Expand Down

0 comments on commit bb7e03b

Please sign in to comment.