{"thread":{"id":"15145","subject":"[JGIT PATCH 1/4] Simplify and micro-optimize WorkingTreeIterator.ENTRY_CMP","startedAt":"2008-08-21T20:57:35Z","lastAt":"2008-08-22T23:07:05Z","messageCount":6,"participants":["Shawn O. Pearce","Robin Rosenberg"],"isPatch":true,"patchVersion":1,"patchTotal":4},"messages":[{"id":"88037","messageId":"1219352258-15431-1-git-send-email-spearce@spearce.org","threadId":"15145","inReplyTo":null,"subject":"[JGIT PATCH 1/4] Simplify and micro-optimize WorkingTreeIterator.ENTRY_CMP","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2008-08-21T20:57:35Z","receivedAt":"2008-08-21T20:57:35Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"We already did this simplification work to AbstractTreeIterator's\npathCompare method, and this is based upon that same structure.\n\nSigned-off-by: Shawn O. Pearce <spearce@spearce.org>\n---\n .../spearce/jgit/treewalk/WorkingTreeIterator.java |   44 ++-----------------\n 1 files changed, 5 insertions(+), 39 deletions(-)\n\ndiff --git a/org.spearce.jgit/src/org/spearce/jgit/treewalk/WorkingTreeIterator.java b/org.spearce.jgit/src/org/spearce/jgit/treewalk/WorkingTreeIterator.java\nindex c6664f5..6fce150 100644\n--- a/org.spearce.jgit/src/org/spearce/jgit/treewalk/WorkingTreeIterator.java\n+++ b/org.spearce.jgit/src/org/spearce/jgit/treewalk/WorkingTreeIterator.java\n@@ -297,45 +297,11 @@ public int compare(final Entry o1, final Entry o2) {\n \t\t\t\t\treturn cmp;\n \t\t\t}\n \n-\t\t\tif (cPos < aLen) {\n-\t\t\t\tfinal int aj = a[cPos] & 0xff;\n-\t\t\t\tfinal int lastb = lastPathChar(o2);\n-\t\t\t\tif (aj < lastb)\n-\t\t\t\t\treturn -1;\n-\t\t\t\telse if (aj > lastb)\n-\t\t\t\t\treturn 1;\n-\t\t\t\telse if (cPos == aLen - 1)\n-\t\t\t\t\treturn 0;\n-\t\t\t\telse\n-\t\t\t\t\treturn -1;\n-\t\t\t}\n-\n-\t\t\tif (cPos < bLen) {\n-\t\t\t\tfinal int bk = b[cPos] & 0xff;\n-\t\t\t\tfinal int lasta = lastPathChar(o1);\n-\t\t\t\tif (lasta < bk)\n-\t\t\t\t\treturn -1;\n-\t\t\t\telse if (lasta > bk)\n-\t\t\t\t\treturn 1;\n-\t\t\t\telse if (cPos == bLen - 1)\n-\t\t\t\t\treturn 0;\n-\t\t\t\telse\n-\t\t\t\t\treturn 1;\n-\t\t\t}\n-\n-\t\t\tfinal int lasta = lastPathChar(o1);\n-\t\t\tfinal int lastb = lastPathChar(o2);\n-\t\t\tif (lasta < lastb)\n-\t\t\t\treturn -1;\n-\t\t\telse if (lasta > lastb)\n-\t\t\t\treturn 1;\n-\n-\t\t\tif (aLen == bLen)\n-\t\t\t\treturn 0;\n-\t\t\telse if (aLen < bLen)\n-\t\t\t\treturn -1;\n-\t\t\telse\n-\t\t\t\treturn 1;\n+\t\t\tif (cPos < aLen)\n+\t\t\t\treturn (a[cPos] & 0xff) - lastPathChar(o2);\n+\t\t\tif (cPos < bLen)\n+\t\t\t\treturn lastPathChar(o1) - (b[cPos] & 0xff);\n+\t\t\treturn lastPathChar(o1) - lastPathChar(o2);\n \t\t}\n \t};\n \n-- \n1.6.0.112.g9c75\n"},{"id":"88038","messageId":"1219352258-15431-2-git-send-email-spearce@spearce.org","threadId":"15145","inReplyTo":"1219352258-15431-1-git-send-email-spearce@spearce.org","subject":"[JGIT PATCH 2/4] Support post order tree handling during TreeWalk","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2008-08-21T20:57:36Z","receivedAt":"2008-08-21T20:57:36Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Some (rare) applications need to handle a tree after they have dealt\nwith the tree's children.  An example of such an application is one\nimplementing rmdir, where the directory should be cleaned up after\nall files/subdirectories were already removed.\n\nSigned-off-by: Shawn O. Pearce <spearce@spearce.org>\n---\n .../jgit/treewalk/PostOrderTreeWalkTest.java       |  189 ++++++++++++++++++++\n .../src/org/spearce/jgit/treewalk/TreeWalk.java    |   53 ++++++-\n 2 files changed, 241 insertions(+), 1 deletions(-)\n create mode 100644 org.spearce.jgit.test/tst/org/spearce/jgit/treewalk/PostOrderTreeWalkTest.java\n\ndiff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/treewalk/PostOrderTreeWalkTest.java b/org.spearce.jgit.test/tst/org/spearce/jgit/treewalk/PostOrderTreeWalkTest.java\nnew file mode 100644\nindex 0000000..145a56e\n--- /dev/null\n+++ b/org.spearce.jgit.test/tst/org/spearce/jgit/treewalk/PostOrderTreeWalkTest.java\n@@ -0,0 +1,189 @@\n+/*\n+ * Copyright (C) 2008, Google Inc.\n+ *\n+ * All rights reserved.\n+ *\n+ * Redistribution and use in source and binary forms, with or\n+ * without modification, are permitted provided that the following\n+ * conditions are met:\n+ *\n+ * - Redistributions of source code must retain the above copyright\n+ *   notice, this list of conditions and the following disclaimer.\n+ *\n+ * - Redistributions in binary form must reproduce the above\n+ *   copyright notice, this list of conditions and the following\n+ *   disclaimer in the documentation and/or other materials provided\n+ *   with the distribution.\n+ *\n+ * - Neither the name of the Git Development Community nor the\n+ *   names of its contributors may be used to endorse or promote\n+ *   products derived from this software without specific prior\n+ *   written permission.\n+ *\n+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND\n+ * CONTRIBUTORS \"AS IS\" AND ANY EXPRESS OR IMPLIED WARRANTIES,\n+ * INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES\n+ * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE\n+ * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR\n+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,\n+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT\n+ * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;\n+ * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER\n+ * CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT,\n+ * STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)\n+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF\n+ * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.\n+ */\n+\n+package org.spearce.jgit.treewalk;\n+\n+import java.io.ByteArrayInputStream;\n+\n+import org.spearce.jgit.dircache.DirCache;\n+import org.spearce.jgit.dircache.DirCacheBuilder;\n+import org.spearce.jgit.dircache.DirCacheEntry;\n+import org.spearce.jgit.dircache.DirCacheIterator;\n+import org.spearce.jgit.lib.Constants;\n+import org.spearce.jgit.lib.FileMode;\n+import org.spearce.jgit.lib.ObjectWriter;\n+import org.spearce.jgit.lib.RepositoryTestCase;\n+\n+import static org.spearce.jgit.lib.FileMode.REGULAR_FILE;\n+import static org.spearce.jgit.lib.FileMode.TREE;\n+\n+public class PostOrderTreeWalkTest extends RepositoryTestCase {\n+\tpublic void testInitialize_NoPostOrder() throws Exception {\n+\t\tfinal TreeWalk tw = new TreeWalk(db);\n+\t\tassertFalse(tw.isPostOrderTraversal());\n+\t}\n+\n+\tpublic void testInitialize_TogglePostOrder() throws Exception {\n+\t\tfinal TreeWalk tw = new TreeWalk(db);\n+\t\tassertFalse(tw.isPostOrderTraversal());\n+\t\ttw.setPostOrderTraversal(true);\n+\t\tassertTrue(tw.isPostOrderTraversal());\n+\t\ttw.setPostOrderTraversal(false);\n+\t\tassertFalse(tw.isPostOrderTraversal());\n+\t}\n+\n+\tpublic void testResetDoesNotAffectPostOrder() throws Exception {\n+\t\tfinal TreeWalk tw = new TreeWalk(db);\n+\t\ttw.setPostOrderTraversal(true);\n+\t\tassertTrue(tw.isPostOrderTraversal());\n+\t\ttw.reset();\n+\t\tassertTrue(tw.isPostOrderTraversal());\n+\n+\t\ttw.setPostOrderTraversal(false);\n+\t\tassertFalse(tw.isPostOrderTraversal());\n+\t\ttw.reset();\n+\t\tassertFalse(tw.isPostOrderTraversal());\n+\t}\n+\n+\tpublic void testNoPostOrder() throws Exception {\n+\t\tfinal DirCache tree = DirCache.read(db);\n+\t\t{\n+\t\t\tfinal DirCacheBuilder b = tree.builder();\n+\n+\t\t\tb.add(makeFile(\"a\"));\n+\t\t\tb.add(makeFile(\"b/c\"));\n+\t\t\tb.add(makeFile(\"b/d\"));\n+\t\t\tb.add(makeFile(\"q\"));\n+\n+\t\t\tb.finish();\n+\t\t\tassertEquals(4, tree.getEntryCount());\n+\t\t}\n+\n+\t\tfinal TreeWalk tw = new TreeWalk(db);\n+\t\ttw.reset();\n+\t\ttw.setPostOrderTraversal(false);\n+\t\ttw.addTree(new DirCacheIterator(tree));\n+\n+\t\tassertModes(\"a\", REGULAR_FILE, tw);\n+\t\tassertModes(\"b\", TREE, tw);\n+\t\tassertTrue(tw.isSubtree());\n+\t\tassertFalse(tw.isPostChildren());\n+\t\ttw.enterSubtree();\n+\t\tassertModes(\"b/c\", REGULAR_FILE, tw);\n+\t\tassertModes(\"b/d\", REGULAR_FILE, tw);\n+\t\tassertModes(\"q\", REGULAR_FILE, tw);\n+\t}\n+\n+\tpublic void testWithPostOrder_EnterSubtree() throws Exception {\n+\t\tfinal DirCache tree = DirCache.read(db);\n+\t\t{\n+\t\t\tfinal DirCacheBuilder b = tree.builder();\n+\n+\t\t\tb.add(makeFile(\"a\"));\n+\t\t\tb.add(makeFile(\"b/c\"));\n+\t\t\tb.add(makeFile(\"b/d\"));\n+\t\t\tb.add(makeFile(\"q\"));\n+\n+\t\t\tb.finish();\n+\t\t\tassertEquals(4, tree.getEntryCount());\n+\t\t}\n+\n+\t\tfinal TreeWalk tw = new TreeWalk(db);\n+\t\ttw.reset();\n+\t\ttw.setPostOrderTraversal(true);\n+\t\ttw.addTree(new DirCacheIterator(tree));\n+\n+\t\tassertModes(\"a\", REGULAR_FILE, tw);\n+\n+\t\tassertModes(\"b\", TREE, tw);\n+\t\tassertTrue(tw.isSubtree());\n+\t\tassertFalse(tw.isPostChildren());\n+\t\ttw.enterSubtree();\n+\t\tassertModes(\"b/c\", REGULAR_FILE, tw);\n+\t\tassertModes(\"b/d\", REGULAR_FILE, tw);\n+\n+\t\tassertModes(\"b\", TREE, tw);\n+\t\tassertTrue(tw.isSubtree());\n+\t\tassertTrue(tw.isPostChildren());\n+\n+\t\tassertModes(\"q\", REGULAR_FILE, tw);\n+\t}\n+\n+\tpublic void testWithPostOrder_NoEnterSubtree() throws Exception {\n+\t\tfinal DirCache tree = DirCache.read(db);\n+\t\t{\n+\t\t\tfinal DirCacheBuilder b = tree.builder();\n+\n+\t\t\tb.add(makeFile(\"a\"));\n+\t\t\tb.add(makeFile(\"b/c\"));\n+\t\t\tb.add(makeFile(\"b/d\"));\n+\t\t\tb.add(makeFile(\"q\"));\n+\n+\t\t\tb.finish();\n+\t\t\tassertEquals(4, tree.getEntryCount());\n+\t\t}\n+\n+\t\tfinal TreeWalk tw = new TreeWalk(db);\n+\t\ttw.reset();\n+\t\ttw.setPostOrderTraversal(true);\n+\t\ttw.addTree(new DirCacheIterator(tree));\n+\n+\t\tassertModes(\"a\", REGULAR_FILE, tw);\n+\n+\t\tassertModes(\"b\", TREE, tw);\n+\t\tassertTrue(tw.isSubtree());\n+\t\tassertFalse(tw.isPostChildren());\n+\n+\t\tassertModes(\"q\", REGULAR_FILE, tw);\n+\t}\n+\n+\tprivate DirCacheEntry makeFile(final String path) throws Exception {\n+\t\tfinal byte[] pathBytes = Constants.encode(path);\n+\t\tfinal DirCacheEntry ent = new DirCacheEntry(path);\n+\t\tent.setFileMode(REGULAR_FILE);\n+\t\tent.setObjectId(new ObjectWriter(db).computeBlobSha1(pathBytes.length,\n+\t\t\t\tnew ByteArrayInputStream(pathBytes)));\n+\t\treturn ent;\n+\t}\n+\n+\tprivate static void assertModes(final String path, final FileMode mode0,\n+\t\t\tfinal TreeWalk tw) throws Exception {\n+\t\tassertTrue(\"has \" + path, tw.next());\n+\t\tassertEquals(path, tw.getPathString());\n+\t\tassertEquals(mode0, tw.getFileMode(0));\n+\t}\n+}\ndiff --git a/org.spearce.jgit/src/org/spearce/jgit/treewalk/TreeWalk.java b/org.spearce.jgit/src/org/spearce/jgit/treewalk/TreeWalk.java\nindex 7a09878..6d0ef02 100644\n--- a/org.spearce.jgit/src/org/spearce/jgit/treewalk/TreeWalk.java\n+++ b/org.spearce.jgit/src/org/spearce/jgit/treewalk/TreeWalk.java\n@@ -147,10 +147,14 @@ public static TreeWalk forPath(final Repository db, final String path,\n \n \tprivate boolean recursive;\n \n+\tprivate boolean postOrderTraversal;\n+\n \tprivate int depth;\n \n \tprivate boolean advance;\n \n+\tprivate boolean postChildren;\n+\n \tAbstractTreeIterator currentHead;\n \n \t/**\n@@ -235,6 +239,36 @@ public void setRecursive(final boolean b) {\n \t\trecursive = b;\n \t}\n \n+\t/**\n+\t * Does this walker return a tree entry after it exits the subtree?\n+\t * <p>\n+\t * If post order traversal is enabled then the walker will return a subtree\n+\t * after it has returned the last entry within that subtree. This may cause\n+\t * a subtree to be seen by the application twice if {@link #isRecursive()}\n+\t * is false, as the application will see it once, call\n+\t * {@link #enterSubtree()}, and then see it again as it leaves the subtree.\n+\t * <p>\n+\t * If an application does not enable {@link #isRecursive()} and it does not\n+\t * call {@link #enterSubtree()} then the tree is returned only once as none\n+\t * of the children were processed.\n+\t * \n+\t * @return true if subtrees are returned after entries within the subtree.\n+\t */\n+\tpublic boolean isPostOrderTraversal() {\n+\t\treturn postOrderTraversal;\n+\t}\n+\n+\t/**\n+\t * Set the walker to return trees after their children.\n+\t * \n+\t * @param b\n+\t *            true to get trees after their children.\n+\t * @see #isPostOrderTraversal()\n+\t */\n+\tpublic void setPostOrderTraversal(final boolean b) {\n+\t\tpostOrderTraversal = b;\n+\t}\n+\n \t/** Reset this walker so new tree iterators can be added to it. */\n \tpublic void reset() {\n \t\ttrees = new AbstractTreeIterator[0];\n@@ -379,6 +413,7 @@ public boolean next() throws MissingObjectException,\n \t\ttry {\n \t\t\tif (advance) {\n \t\t\t\tadvance = false;\n+\t\t\t\tpostChildren = false;\n \t\t\t\tpopEntriesEqual();\n \t\t\t}\n \n@@ -387,6 +422,12 @@ public boolean next() throws MissingObjectException,\n \t\t\t\tif (t.eof()) {\n \t\t\t\t\tif (depth > 0) {\n \t\t\t\t\t\texitSubtree();\n+\t\t\t\t\t\tif (postOrderTraversal) {\n+\t\t\t\t\t\t\tadvance = true;\n+\t\t\t\t\t\t\tpostChildren = true;\n+\t\t\t\t\t\t\treturn true;\n+\t\t\t\t\t\t}\n+\t\t\t\t\t\tpopEntriesEqual();\n \t\t\t\t\t\tcontinue;\n \t\t\t\t\t}\n \t\t\t\t\treturn false;\n@@ -587,6 +628,17 @@ public boolean isSubtree() {\n \t}\n \n \t/**\n+\t * Is the current entry a subtree returned after its children?\n+\t * \n+\t * @return true if the current node is a tree that has been returned after\n+\t *         its children were already processed.\n+\t * @see #isPostOrderTraversal()\n+\t */\n+\tpublic boolean isPostChildren() {\n+\t\treturn postChildren && isSubtree();\n+\t}\n+\n+\t/**\n \t * Enter into the current subtree.\n \t * <p>\n \t * If the current entry is a subtree this method arranges for its children\n@@ -675,7 +727,6 @@ private void exitSubtree() throws CorruptObjectException {\n \t\tfor (int i = 0; i < trees.length; i++)\n \t\t\ttrees[i] = trees[i].parent;\n \t\tcurrentHead = min();\n-\t\tpopEntriesEqual();\n \t}\n \n \tprivate CanonicalTreeParser parserFor(final ObjectId id)\n-- \n1.6.0.112.g9c75\n"},{"id":"88039","messageId":"1219352258-15431-3-git-send-email-spearce@spearce.org","threadId":"15145","inReplyTo":"1219352258-15431-2-git-send-email-spearce@spearce.org","subject":"[JGIT PATCH 3/4] Micro-optimize TreeWalk's exitSubtree implementation","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2008-08-21T20:57:37Z","receivedAt":"2008-08-21T20:57:37Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Rather than recomputing the min over again we can take the hint that\nthe prior min (the one that describes the tree we just left) must be\none that matches itself.  There may be more than one such case, as a\nmin could be found and match itself and later another min is found.\nSo we fall back into a pathCompare if we identify more than one.\n\nSigned-off-by: Shawn O. Pearce <spearce@spearce.org>\n---\n .../src/org/spearce/jgit/treewalk/TreeWalk.java    |   12 ++++++++++--\n 1 files changed, 10 insertions(+), 2 deletions(-)\n\ndiff --git a/org.spearce.jgit/src/org/spearce/jgit/treewalk/TreeWalk.java b/org.spearce.jgit/src/org/spearce/jgit/treewalk/TreeWalk.java\nindex 6d0ef02..ef27e4e 100644\n--- a/org.spearce.jgit/src/org/spearce/jgit/treewalk/TreeWalk.java\n+++ b/org.spearce.jgit/src/org/spearce/jgit/treewalk/TreeWalk.java\n@@ -722,11 +722,19 @@ void skipEntriesEqual() throws CorruptObjectException {\n \t\t}\n \t}\n \n-\tprivate void exitSubtree() throws CorruptObjectException {\n+\tprivate void exitSubtree() {\n \t\tdepth--;\n \t\tfor (int i = 0; i < trees.length; i++)\n \t\t\ttrees[i] = trees[i].parent;\n-\t\tcurrentHead = min();\n+\n+\t\tAbstractTreeIterator minRef = null;\n+\t\tfor (final AbstractTreeIterator t : trees) {\n+\t\t\tif (t.matches != t)\n+\t\t\t\tcontinue;\n+\t\t\tif (minRef == null || t.pathCompare(minRef) < 0)\n+\t\t\t\tminRef = t;\n+\t\t}\n+\t\tcurrentHead = minRef;\n \t}\n \n \tprivate CanonicalTreeParser parserFor(final ObjectId id)\n-- \n1.6.0.112.g9c75\n"},{"id":"88040","messageId":"1219352258-15431-4-git-send-email-spearce@spearce.org","threadId":"15145","inReplyTo":"1219352258-15431-3-git-send-email-spearce@spearce.org","subject":"[JGIT PATCH 4/4] Add getNameString to TreeWalk to get only the last path component","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2008-08-21T20:57:38Z","receivedAt":"2008-08-21T20:57:38Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Some applications may need to see only the name of the current\nentry within its parent tree and not require the full path from\nthe root of the repository.\n\nSigned-off-by: Shawn O. Pearce <spearce@spearce.org>\n---\n .../src/org/spearce/jgit/treewalk/TreeWalk.java    |   18 ++++++++++++++++++\n 1 files changed, 18 insertions(+), 0 deletions(-)\n\ndiff --git a/org.spearce.jgit/src/org/spearce/jgit/treewalk/TreeWalk.java b/org.spearce.jgit/src/org/spearce/jgit/treewalk/TreeWalk.java\nindex ef27e4e..53b94d2 100644\n--- a/org.spearce.jgit/src/org/spearce/jgit/treewalk/TreeWalk.java\n+++ b/org.spearce.jgit/src/org/spearce/jgit/treewalk/TreeWalk.java\n@@ -548,6 +548,24 @@ public boolean idEqual(final int nthA, final int nthB) {\n \t}\n \n \t/**\n+\t * Get the current entry's name within its parent tree.\n+\t * <p>\n+\t * This method is not very efficient and is primarily meant for debugging\n+\t * and final output generation. Applications should try to avoid calling it,\n+\t * and if invoked do so only once per interesting entry, where the name is\n+\t * absolutely required for correct function.\n+\t * \n+\t * @return name of the current entry within the parent tree (or directory).\n+\t *         The name never includes a '/'.\n+\t */\n+\tpublic String getNameString() {\n+\t\tfinal AbstractTreeIterator t = currentHead;\n+\t\tfinal int off = t.pathOffset;\n+\t\tfinal int end = t.pathLen;\n+\t\treturn RawParseUtils.decode(Constants.CHARSET, t.path, off, end);\n+\t}\n+\n+\t/**\n \t * Get the current entry's complete path.\n \t * <p>\n \t * This method is not very efficient and is primarily meant for debugging\n-- \n1.6.0.112.g9c75\n"},{"id":"88193","messageId":"200808222245.08814.robin.rosenberg.lists@dewire.com","threadId":"15145","inReplyTo":"1219352258-15431-1-git-send-email-spearce@spearce.org","subject":"Re: [JGIT PATCH 1/4] Simplify and micro-optimize WorkingTreeIterator.ENTRY_CMP","fromName":"Robin Rosenberg","fromEmail":"robin.rosenberg.lists@dewire.com","sentAt":"2008-08-22T20:45:08Z","receivedAt":"2008-08-22T20:45:08Z","isPatch":true,"sender":{"key":"robin.rosenberg@dewire.com","avatar":"https://avatars.githubusercontent.com/u/46357?v=4"},"body":"torsdagen den 21 augusti 2008 22.57.35 skrev Shawn O. Pearce:\n> We already did this simplification work to AbstractTreeIterator's\n> pathCompare method, and this is based upon that same structure.\n\nI noticed we have very bad code coverage in unit tests in this area of the code. \nMaybe we should increase the coverage before optimizing. I'll accept the patches\nanyway and pray they are ok.\n\n-- robin\n"},{"id":"88226","messageId":"20080822230705.GO3483@spearce.org","threadId":"15145","inReplyTo":"200808222245.08814.robin.rosenberg.lists@dewire.com","subject":"Re: [JGIT PATCH 1/4] Simplify and micro-optimize WorkingTreeIterator.ENTRY_CMP","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2008-08-22T23:07:05Z","receivedAt":"2008-08-22T23:07:05Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Robin Rosenberg <robin.rosenberg.lists@dewire.com> wrote:\n> torsdagen den 21 augusti 2008 22.57.35 skrev Shawn O. Pearce:\n> > We already did this simplification work to AbstractTreeIterator's\n> > pathCompare method, and this is based upon that same structure.\n> \n> I noticed we have very bad code coverage in unit tests in this area of the code. \n> Maybe we should increase the coverage before optimizing. I'll accept the patches\n> anyway and pray they are ok.\n\nYea, I didn't go into here just to optimize the code.  But I looked\nat it and said \"WTF was I smoking when I wrote that?  Must have\nbeen some good quality stuff!\".\n\nI was in there studying the method because I wanted to call it\nfrom someplace else, and then realized how crappy it was.  So as I\nsimplified it, I noticed we didn't have to do as many compares to\ncome up with the same result.\n\nI'm working on improving test coverage.  Slowly.  But I am trying\nto spend at least a few hours a week on it.  I know we are really,\nreally bad here.  It will get better.\n\n-- \nShawn.\n"}]}