[ctree] change the ManifestIterator to return ManifestEntry *

Summary: This allows us to modify the ManifestEntry stored in memory.  This also requires us to remove the const qualifier in a number of places.

Test Plan: `PYTHONPATH=~/work/mercurial/facebook-hg-rpms/remotefilelog:~/work/mercurial/facebook-hg-rpms/fb-hgext/:~/work/mercurial/facebook-hg-rpms/remotenames/:~/work/mercurial/facebook-hg-rpms/lz4revlog/ /opt/local/bin/python2.7 ~/work/mercurial/facebook-hg-rpms/hg-crew/hg --config extensions.perftest=~/work/mercurial/facebook-hg-rpms/remotefilelog/tests/perftest.py testtree --kind flat,ctree,fast --test fulliter,diff,find --build "master~5::master"`

Reviewers: #fastmanifest, durham

Reviewed By: durham

Subscribers: mitrandir

Differential Revision: https://phabricator.intern.facebook.com/D3752366

Tasks: 12818084

Signature: t1:3752366:1471891101:1d42a04d85b7e8db34644dc8fbf1bb3481fbb7bc

remotefilelog/ctreemanifest/manifest.cpp

@@ -22,7 +22,7 @@ Manifest::Manifest(PythonObj &rawobj) :
   }
 }
 
-ManifestIterator Manifest::getIterator() const {
+ManifestIterator Manifest::getIterator() {
   return ManifestIterator(this->entries.begin(), this->entries.end());
 }
 
@@ -76,28 +76,28 @@ ManifestEntry& Manifest::addChild(
 }
 
 ManifestIterator::ManifestIterator(
-    std::list<ManifestEntry>::const_iterator iterator,
+    std::list<ManifestEntry>::iterator iterator,
     std::list<ManifestEntry>::const_iterator end) :
     iterator(iterator), end(end) {
 }
 
-bool ManifestIterator::next(ManifestEntry *entry) {
+bool ManifestIterator::next(ManifestEntry **entry) {
   if (this->isfinished()) {
     return false;
   }
 
-  *entry = *this->iterator;
+  *entry = &(*this->iterator);
   this->iterator++;
 
   return true;
 }
 
-ManifestEntry ManifestIterator::currentvalue() const {
+ManifestEntry *ManifestIterator::currentvalue() const {
   if (this->isfinished()) {
     throw std::logic_error("iterator has no current value");
   }
 
-  return *iterator;
+  return &(*iterator);
 }
 
 bool ManifestIterator::isfinished() const {

remotefilelog/ctreemanifest/manifest.h

@@ -46,7 +46,7 @@ class Manifest {
 
     Manifest(PythonObj &rawobj);
 
-    ManifestIterator getIterator() const;
+    ManifestIterator getIterator();
 
     /**
      * Returns an iterator correctly positioned for a child of a given
@@ -76,19 +76,19 @@ class Manifest {
  */
 class ManifestIterator {
   private:
-    std::list<ManifestEntry>::const_iterator iterator;
+    std::list<ManifestEntry>::iterator iterator;
     std::list<ManifestEntry>::const_iterator end;
   public:
     ManifestIterator() {
     }
 
     ManifestIterator(
-        std::list<ManifestEntry>::const_iterator iterator,
+        std::list<ManifestEntry>::iterator iterator,
         std::list<ManifestEntry>::const_iterator end);
 
-    bool next(ManifestEntry *entry);
+    bool next(ManifestEntry **entry);
 
-    ManifestEntry currentvalue() const;
+    ManifestEntry *currentvalue() const;
 
     bool isfinished() const;
 };

remotefilelog/ctreemanifest/py-treemanifest.cpp

@@ -165,8 +165,8 @@ class DiffEntry {
  * Helper function that performs the actual recursion on the tree entries.
  */
 static void treemanifest_diffrecurse(
-    const Manifest *selfmf,
-    const Manifest *othermf,
+    Manifest *selfmf,
+    Manifest *othermf,
     std::string *path,
     const PythonObj &diff,
     const ManifestFetcher &fetcher) {
@@ -184,57 +184,57 @@ static void treemanifest_diffrecurse(
   while (!selfiter.isfinished() || !otheriter.isfinished()) {
     int cmp = 0;
 
-    ManifestEntry selfentry;
+    ManifestEntry *selfentry;
     std::string selfbinnode;
     if (!selfiter.isfinished()) {
       cmp--;
       selfentry = selfiter.currentvalue();
-      selfbinnode = binfromhex(selfentry.node);
+      selfbinnode = binfromhex(selfentry->node);
     }
 
-    ManifestEntry otherentry;
+    ManifestEntry *otherentry;
     std::string otherbinnode;
     if (!otheriter.isfinished()) {
       cmp++;
       otherentry = otheriter.currentvalue();
-      otherbinnode = binfromhex(otherentry.node);
+      otherbinnode = binfromhex(otherentry->node);
     }
 
     // If both sides are present, cmp == 0, so do a filename comparison
     if (cmp == 0) {
-      cmp = strcmp(selfentry.filename, otherentry.filename);
+      cmp = strcmp(selfentry->filename, otherentry->filename);
     }
 
     int originalpathsize = path->size();
     if (cmp < 0) {
       // selfentry should be processed first and only exists in self
-      selfentry.appendtopath(*path);
-      if (selfentry.isdirectory()) {
-        Manifest *selfchildmanifest = selfentry.get_manifest(
+      selfentry->appendtopath(*path);
+      if (selfentry->isdirectory()) {
+        Manifest *selfchildmanifest = selfentry->get_manifest(
             fetcher, path->c_str(), path->size());
         treemanifest_diffrecurse(selfchildmanifest, NULL, path, diff, fetcher);
       } else {
-        DiffEntry entry(&selfbinnode, selfentry.flag, NULL, NULL);
+        DiffEntry entry(&selfbinnode, selfentry->flag, NULL, NULL);
         entry.addtodiff(diff, *path);
       }
       selfiter.next(&selfentry);
     } else if (cmp > 0) {
       // otherentry should be processed first and only exists in other
-      otherentry.appendtopath(*path);
-      if (otherentry.isdirectory()) {
-        Manifest *otherchildmanifest = otherentry.get_manifest(
+      otherentry->appendtopath(*path);
+      if (otherentry->isdirectory()) {
+        Manifest *otherchildmanifest = otherentry->get_manifest(
             fetcher, path->c_str(), path->size());
         treemanifest_diffrecurse(NULL, otherchildmanifest, path, diff, fetcher);
       } else {
-        DiffEntry entry(NULL, NULL, &otherbinnode, otherentry.flag);
+        DiffEntry entry(NULL, NULL, &otherbinnode, otherentry->flag);
         entry.addtodiff(diff, *path);
       }
       otheriter.next(&otherentry);
     } else {
       // Filenames match - now compare directory vs file
-      if (selfentry.isdirectory() && otherentry.isdirectory()) {
+      if (selfentry->isdirectory() && otherentry->isdirectory()) {
         // Both are directories - recurse
-        selfentry.appendtopath(*path);
+        selfentry->appendtopath(*path);
 
         if (selfbinnode != otherbinnode) {
           Manifest *selfchildmanifest = fetcher.get(
@@ -253,10 +253,10 @@ static void treemanifest_diffrecurse(
         }
         selfiter.next(&selfentry);
         otheriter.next(&otherentry);
-      } else if (selfentry.isdirectory() && !otherentry.isdirectory()) {
+      } else if (selfentry->isdirectory() && !otherentry->isdirectory()) {
         // self is directory, other is not - process other then self
-        otherentry.appendtopath(*path);
-        DiffEntry entry(NULL, NULL, &otherbinnode, otherentry.flag);
+        otherentry->appendtopath(*path);
+        DiffEntry entry(NULL, NULL, &otherbinnode, otherentry->flag);
         entry.addtodiff(diff, *path);
 
         path->append(1, '/');
@@ -267,10 +267,10 @@ static void treemanifest_diffrecurse(
 
         selfiter.next(&selfentry);
         otheriter.next(&otherentry);
-      } else if (!selfentry.isdirectory() && otherentry.isdirectory()) {
+      } else if (!selfentry->isdirectory() && otherentry->isdirectory()) {
         // self is not directory, other is - process self then other
-        selfentry.appendtopath(*path);
-        DiffEntry entry(&selfbinnode, selfentry.flag, NULL, NULL);
+        selfentry->appendtopath(*path);
+        DiffEntry entry(&selfbinnode, selfentry->flag, NULL, NULL);
         entry.addtodiff(diff, *path);
 
         path->append(1, '/');
@@ -285,13 +285,13 @@ static void treemanifest_diffrecurse(
       } else {
         // both are files
         bool flagsdiffer = (
-          (selfentry.flag && otherentry.flag && *selfentry.flag != *otherentry.flag) ||
-          ((bool)selfentry.flag != (bool)selfentry.flag)
+          (selfentry->flag && otherentry->flag && *selfentry->flag != *otherentry->flag) ||
+          ((bool)selfentry->flag != (bool)selfentry->flag)
         );
 
         if (selfbinnode != otherbinnode || flagsdiffer) {
-          selfentry.appendtopath(*path);
-          DiffEntry entry(&selfbinnode, selfentry.flag, &otherbinnode, otherentry.flag);
+          selfentry->appendtopath(*path);
+          DiffEntry entry(&selfbinnode, selfentry->flag, &otherbinnode, otherentry->flag);
           entry.addtodiff(diff, *path);
         }
 
@@ -480,15 +480,15 @@ static PyObject *fileiter_iterentriesnext(py_fileiter *self) {
       stackframe &frame = iter.frames.back();
       ManifestIterator &iterator = frame.iterator;
 
-      ManifestEntry entry;
+      ManifestEntry* entry;
       iterator.next(&entry);
 
       // If a directory, push it and loop again
-      if (entry.isdirectory()) {
-        iter.path.append(entry.filename, entry.filenamelen);
+      if (entry->isdirectory()) {
+        iter.path.append(entry->filename, entry->filenamelen);
         iter.path.append(1, '/');
 
-        Manifest *submanifest = entry.get_manifest(iter.fetcher,
+        Manifest *submanifest = entry->get_manifest(iter.fetcher,
             iter.path.c_str(), iter.path.size());
 
         // TODO: memory cleanup here is probably broken.
@@ -497,7 +497,7 @@ static PyObject *fileiter_iterentriesnext(py_fileiter *self) {
       } else {
         // If a file, yield it
         int oldpathsize = iter.path.size();
-        iter.path.append(entry.filename, entry.filenamelen);
+        iter.path.append(entry->filename, entry->filenamelen);
         PyObject* result = PyString_FromStringAndSize(iter.path.c_str(), iter.path.length());
         if (!result) {
           PyErr_NoMemory();

remotefilelog/ctreemanifest/treemanifest.cpp

@@ -28,7 +28,7 @@ void _treemanifest_find(
     // TODO: need to attach this manifest to the parent Manifest object.
 
     ManifestIterator mfiterator = manifest->getIterator();
-    ManifestEntry entry;
+    ManifestEntry *entry;
     bool recurse = false;
 
     // Loop over the contents of the current directory looking for the
@@ -36,16 +36,17 @@ void _treemanifest_find(
     while (mfiterator.next(&entry)) {
       // If the current entry matches the query file/directory, either recurse,
       // return, or abort.
-      if (wordlen == entry.filenamelen && strncmp(word, entry.filename, wordlen) == 0) {
+      if (wordlen == entry->filenamelen &&
+          strncmp(word, entry->filename, wordlen) == 0) {
         // If this is the last entry in the query path, either return or abort
         if (pathiter.isfinished()) {
           // If it's a file, it's our result
-          if (!entry.isdirectory()) {
-            resultnode->assign(binfromhex(entry.node));
-            if (entry.flag == NULL) {
+          if (!entry->isdirectory()) {
+            resultnode->assign(binfromhex(entry->node));
+            if (entry->flag == NULL) {
               *resultflag = '\0';
             } else {
-              *resultflag = *entry.flag;
+              *resultflag = *entry->flag;
             }
             return;
           } else {
@@ -56,9 +57,9 @@ void _treemanifest_find(
 
         // If there's more in the query, either recurse or give up
         curpathlen = curpathlen + wordlen + 1;
-        if (entry.isdirectory() && filename.length() > curpathlen) {
+        if (entry->isdirectory() && filename.length() > curpathlen) {
           curnode.erase();
-          curnode.append(binfromhex(entry.node));
+          curnode.append(binfromhex(entry->node));
           recurse = true;
           break;
         } else {

remotefilelog/ctreemanifest/treemanifest.h

@@ -36,10 +36,10 @@ struct treemanifest {
  * Represents a single stack frame in an iteration of the contents of the tree.
  */
 struct stackframe {
-  const Manifest *manifest;
+  Manifest *manifest;
   ManifestIterator iterator;
 
-  stackframe(const Manifest *manifest) :
+  stackframe(Manifest *manifest) :
       manifest(manifest),
       iterator(manifest->getIterator()) {
   }