{"thread":{"id":"18696","subject":"[EGIT PATCH 1/5] Make the equals method work for AnyObjectId, not just ObjectId","startedAt":"2009-04-02T18:46:26Z","lastAt":"2009-04-05T21:43:32Z","messageCount":11,"participants":["Robin Rosenberg","Shawn O. Pearce"],"isPatch":true,"patchVersion":1,"patchTotal":5},"messages":[{"id":"110232","messageId":"1238697991-12990-1-git-send-email-robin.rosenberg@dewire.com","threadId":"18696","inReplyTo":null,"subject":"[EGIT PATCH 0/5] Quickdiff improvements","fromName":"Robin Rosenberg","fromEmail":"robin.rosenberg@dewire.com","sentAt":"2009-04-02T18:46:26Z","receivedAt":"2009-04-02T18:46:26Z","isPatch":true,"sender":{"key":"robin.rosenberg@dewire.com","avatar":"https://avatars.githubusercontent.com/u/46357?v=4"},"body":"I noticed Eclipse could hang for long period, especially\nduring rebase. This series fixes that.\n\n-- robin\n\nRobin Rosenberg (5):\n Make the equals method work for AnyObjectId, not just ObjectId\n quickdiff - Don't add GitDocument as repository listener more than\n once\n Move dcocument to repository mapping to GitDocument\n Update Quickdiff tracing statements\n Cache resolved ids in quickdiff document for faster update\n\n .../egit/ui/internal/decorators/GitDocument.java | 104 +++++++++++++++++---\n .../internal/decorators/GitQuickDiffProvider.java | 10 +--\n .../src/org/spearce/jgit/lib/AnyObjectId.java | 4 +-\n .../src/org/spearce/jgit/lib/Repository.java | 2 +\n .../src/org/spearce/jgit/revwalk/RevObject.java | 2 +-\n 5 files changed, 97 insertions(+), 25 deletions(-)\n"},{"id":"110231","messageId":"1238697991-12990-2-git-send-email-robin.rosenberg@dewire.com","threadId":"18696","inReplyTo":"1238697991-12990-1-git-send-email-robin.rosenberg@dewire.com","subject":"[EGIT PATCH 1/5] Make the equals method work for AnyObjectId, not just ObjectId","fromName":"Robin Rosenberg","fromEmail":"robin.rosenberg@dewire.com","sentAt":"2009-04-02T18:46:27Z","receivedAt":"2009-04-02T18:46:27Z","isPatch":true,"sender":{"key":"robin.rosenberg@dewire.com","avatar":"https://avatars.githubusercontent.com/u/46357?v=4"},"body":"Signed-off-by: Robin Rosenberg \n---\n\nThis has nothing to do with the series, just trying to squeeze it in... The\nquickdiff fixes works fine with our without this patch, but it seems reasonable.\n\n .../src/org/spearce/jgit/lib/AnyObjectId.java | 4 ++--\n .../src/org/spearce/jgit/revwalk/RevObject.java | 2 +-\n 2 files changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/AnyObjectId.java b/org.spearce.jgit/src/org/spearce/jgit/lib/AnyObjectId.java\nindex e2f70ca..2e3a43e 100644\n--- a/org.spearce.jgit/src/org/spearce/jgit/lib/AnyObjectId.java\n+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/AnyObjectId.java\n@@ -248,12 +248,12 @@ public int hashCode() {\n \t * the other id to compare to. May be null.\n \t * @return true only if both ObjectIds have identical bits.\n \t */\n-\tpublic boolean equals(final ObjectId other) {\n+\tpublic boolean equals(final AnyObjectId other) {\n \t\treturn other != null ? equals(this, other) : false;\n \t}\n \n \tpublic boolean equals(final Object o) {\n-\t\treturn equals((ObjectId) o);\n+\t\treturn equals((AnyObjectId) o);\n \t}\n \n \t/**\ndiff --git a/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevObject.java b/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevObject.java\nindex e8fb29f..1a13d0a 100644\n--- a/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevObject.java\n+++ b/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevObject.java\n@@ -75,7 +75,7 @@ public final ObjectId getId() {\n \t}\n \n \t@Override\n-\tpublic final boolean equals(final ObjectId o) {\n+\tpublic final boolean equals(final AnyObjectId o) {\n \t\treturn this == o;\n \t}\n \n-- \n1.6.2.1.345.g89fb\n"},{"id":"110234","messageId":"1238697991-12990-3-git-send-email-robin.rosenberg@dewire.com","threadId":"18696","inReplyTo":"1238697991-12990-2-git-send-email-robin.rosenberg@dewire.com","subject":"[EGIT PATCH 2/5] quickdiff - Don't add GitDocument as repository listener more than once","fromName":"Robin Rosenberg","fromEmail":"robin.rosenberg@dewire.com","sentAt":"2009-04-02T18:46:28Z","receivedAt":"2009-04-02T18:46:28Z","isPatch":true,"sender":{"key":"robin.rosenberg@dewire.com","avatar":"https://avatars.githubusercontent.com/u/46357?v=4"},"body":"Doing so was very costly and led to sessions being locked\nfor a long time while refreshing the reference document used\nby Eclipse's quickdiff feature.\n\nSigned-off-by: Robin Rosenberg \n---\n .../egit/ui/internal/decorators/GitDocument.java | 4 +++-\n .../src/org/spearce/jgit/lib/Repository.java | 2 ++\n 2 files changed, 5 insertions(+), 1 deletions(-)\n\ndiff --git a/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/decorators/GitDocument.java b/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/decorators/GitDocument.java\nindex a9c0c7d..69e9295 100644\n--- a/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/decorators/GitDocument.java\n+++ b/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/decorators/GitDocument.java\n@@ -35,6 +35,9 @@ static GitDocument create(final IResource resource) throws IOException {\n \t\tif (RepositoryProvider.getProvider(resource.getProject()) instanceof GitProvider) {\n \t\t\tret = new GitDocument(resource);\n \t\t\tret.populate();\n+\t\t\tfinal Repository repository = ret.getRepository();\n+\t\t\tif (repository != null)\n+\t\t\t\trepository.addRepositoryChangedListener(ret);\n \t\t}\n \t\treturn ret;\n \t}\n@@ -52,7 +55,6 @@ void populate() throws IOException {\n \t\t\treturn;\n \t\tfinal String gitPath = mapping.getRepoRelativePath(resource);\n \t\tfinal Repository repository = getRepository();\n-\t\trepository.addRepositoryChangedListener(this);\n \t\tString baseline = GitQuickDiffProvider.baseline.get(repository);\n \t\tif (baseline == null)\n \t\t\tbaseline = Constants.HEAD;\ndiff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java b/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java\nindex cfd92b8..0f396ed 100644\n--- a/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java\n+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java\n@@ -1132,6 +1132,7 @@ public File getWorkDir() {\n \t * @param l\n \t */\n \tpublic void addRepositoryChangedListener(final RepositoryListener l) {\n+\t\tassert !listeners.contains(l);\n \t\tlisteners.add(l);\n \t}\n \n@@ -1150,6 +1151,7 @@ public void removeRepositoryChangedListener(final RepositoryListener l) {\n \t * @param l\n \t */\n \tpublic static void addAnyRepositoryChangedListener(final RepositoryListener l) {\n+\t\tassert !allListeners.contains(l);\n \t\tallListeners.add(l);\n \t}\n \n-- \n1.6.2.1.345.g89fb\n"},{"id":"110233","messageId":"1238697991-12990-4-git-send-email-robin.rosenberg@dewire.com","threadId":"18696","inReplyTo":"1238697991-12990-3-git-send-email-robin.rosenberg@dewire.com","subject":"[EGIT PATCH 3/5] Move dcocument to repository mapping to GitDocument","fromName":"Robin Rosenberg","fromEmail":"robin.rosenberg@dewire.com","sentAt":"2009-04-02T18:46:29Z","receivedAt":"2009-04-02T18:46:29Z","isPatch":true,"sender":{"key":"robin.rosenberg@dewire.com","avatar":"https://avatars.githubusercontent.com/u/46357?v=4"},"body":"Signed-off-by: Robin Rosenberg \n---\n .../egit/ui/internal/decorators/GitDocument.java | 23 ++++++++++++++++++-\n .../internal/decorators/GitQuickDiffProvider.java | 8 +------\n 2 files changed, 22 insertions(+), 9 deletions(-)\n\ndiff --git a/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/decorators/GitDocument.java b/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/decorators/GitDocument.java\nindex 69e9295..8c82a55 100644\n--- a/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/decorators/GitDocument.java\n+++ b/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/decorators/GitDocument.java\n@@ -1,5 +1,5 @@\n /*******************************************************************************\n- * Copyright (C) 2008, Robin Rosenberg \n+ * Copyright (C) 2008, 2009 Robin Rosenberg \n *\n * All rights reserved. This program and the accompanying materials\n * are made available under the terms of the Eclipse Public License v1.0\n@@ -8,6 +8,8 @@\n package org.spearce.egit.ui.internal.decorators;\n \n import java.io.IOException;\n+import java.util.Map;\n+import java.util.WeakHashMap;\n \n import org.eclipse.core.resources.IEncodedStorage;\n import org.eclipse.core.resources.IProject;\n@@ -29,6 +31,7 @@\n \n class GitDocument extends Document implements RepositoryListener {\n \tprivate final IResource resource;\n+\tstatic Map doc2repo = new WeakHashMap();\n \n \tstatic GitDocument create(final IResource resource) throws IOException {\n \t\tGitDocument ret = null;\n@@ -44,7 +47,7 @@ static GitDocument create(final IResource resource) throws IOException {\n \n \tprivate GitDocument(IResource resource) {\n \t\tthis.resource = resource;\n-\t\tGitQuickDiffProvider.doc2repo.put(this, getRepository());\n+\t\tGitDocument.doc2repo.put(this, getRepository());\n \t}\n \n \tvoid populate() throws IOException {\n@@ -95,6 +98,7 @@ void populate() throws IOException {\n \t}\n \n \tvoid dispose() {\n+\t\tdoc2repo.remove(this);\n \t\tRepository repository = getRepository();\n \t\tif (repository != null)\n \t\t\trepository.removeRepositoryChangedListener(this);\n@@ -119,4 +123,19 @@ private Repository getRepository() {\n \t\t\treturn mapping.getRepository();\n \t\treturn null;\n \t}\n+\n+\t/**\n+\t * A change occurred to a repository. Update any GitDocument instances\n+\t * referring to such repositories.\n+\t * \n+\t * @param repository Repository which changed \n+\t * @throws IOException\n+\t */\n+\tstatic void refreshRelevant(final Repository repository) throws IOException {\n+\t\tfor (Map.Entry i : doc2repo.entrySet()) {\n+\t\t\tif (i.getValue() == repository) {\n+\t\t\t\ti.getKey().populate();\n+\t\t\t}\n+\t\t}\n+\t}\n }\ndiff --git a/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/decorators/GitQuickDiffProvider.java b/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/decorators/GitQuickDiffProvider.java\nindex 88f5ea0..6c71f3c 100644\n--- a/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/decorators/GitQuickDiffProvider.java\n+++ b/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/decorators/GitQuickDiffProvider.java\n@@ -37,7 +37,6 @@\n \tprivate IResource resource;\n \n \tstatic Map baseline = new WeakHashMap();\n-\tstatic Map doc2repo = new WeakHashMap();\n \n \t/**\n \t * Create the GitQuickDiffProvider instance\n@@ -48,7 +47,6 @@ public GitQuickDiffProvider() {\n \n \tpublic void dispose() {\n \t\tActivator.trace(\"(GitQuickDiffProvider) dispose\");\n-\t\tdoc2repo.remove(document);\n \t\tif (document != null)\n \t\t\tdocument.dispose();\n \t}\n@@ -96,11 +94,7 @@ public void setId(String id) {\n \t */\n \tpublic static void setBaselineReference(final Repository repository, final String baseline) throws IOException {\n \t\tGitQuickDiffProvider.baseline.put(repository, baseline);\n-\t\tfor (Map.Entry i : doc2repo.entrySet()) {\n-\t\t\tif (i.getValue() == repository) {\n-\t\t\t\ti.getKey().populate();\n-\t\t\t}\n-\t\t}\n+\t\tGitDocument.refreshRelevant(repository);\n \t}\n \n }\n-- \n1.6.2.1.345.g89fb\n"},{"id":"110236","messageId":"1238697991-12990-5-git-send-email-robin.rosenberg@dewire.com","threadId":"18696","inReplyTo":"1238697991-12990-4-git-send-email-robin.rosenberg@dewire.com","subject":"[EGIT PATCH 4/5] Update Quickdiff tracing statements","fromName":"Robin Rosenberg","fromEmail":"robin.rosenberg@dewire.com","sentAt":"2009-04-02T18:46:30Z","receivedAt":"2009-04-02T18:46:30Z","isPatch":true,"sender":{"key":"robin.rosenberg@dewire.com","avatar":"https://avatars.githubusercontent.com/u/46357?v=4"},"body":"Signed-off-by: Robin Rosenberg \n---\n .../egit/ui/internal/decorators/GitDocument.java | 9 ++++++---\n .../internal/decorators/GitQuickDiffProvider.java | 2 ++\n 2 files changed, 8 insertions(+), 3 deletions(-)\n\ndiff --git a/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/decorators/GitDocument.java b/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/decorators/GitDocument.java\nindex 8c82a55..347e6fc 100644\n--- a/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/decorators/GitDocument.java\n+++ b/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/decorators/GitDocument.java\n@@ -34,6 +34,7 @@\n \tstatic Map doc2repo = new WeakHashMap();\n \n \tstatic GitDocument create(final IResource resource) throws IOException {\n+\t\tActivator.trace(\"(GitDocument) create: \" + resource);\n \t\tGitDocument ret = null;\n \t\tif (RepositoryProvider.getProvider(resource.getProject()) instanceof GitProvider) {\n \t\t\tret = new GitDocument(resource);\n@@ -51,6 +52,7 @@ private GitDocument(IResource resource) {\n \t}\n \n \tvoid populate() throws IOException {\n+\t\tActivator.trace(\"(GitDocument) populate: \" + resource);\n \t\tset(\"\");\n \t\tfinal IProject project = resource.getProject();\n \t\tRepositoryMapping mapping = RepositoryMapping.getMapping(project);\n@@ -70,7 +72,7 @@ void populate() throws IOException {\n \t\t}\n \t\tTreeEntry blobEnry = baselineTree.findBlobMember(gitPath);\n \t\tif (blobEnry != null) {\n-\t\t\tActivator.trace(\"(GitQuickDiffProvider) compareTo: \" + baseline);\n+\t\t\tActivator.trace(\"(GitDocument) compareTo: \" + baseline);\n \t\t\tObjectLoader loader = repository.openBlob(blobEnry.getId());\n \t\t\tbyte[] bytes = loader.getBytes();\n \t\t\tString charset;\n@@ -91,13 +93,14 @@ void populate() throws IOException {\n \t\t\t// to the content. We don't do that here.\n \t\t\tString s = new String(bytes, charset);\n \t\t\tset(s);\n-\t\t\tActivator.trace(\"(GitQuickDiffProvider) has reference doc, size=\" + s.length() + \" bytes\");\n+\t\t\tActivator.trace(\"(GitDocument) has reference doc, size=\" + s.length() + \" bytes\");\n \t\t} else {\n-\t\t\tActivator.trace(\"(GitQuickDiffProvider) no revision.\");\n+\t\t\tActivator.trace(\"(GitDocument) no revision.\");\n \t\t}\n \t}\n \n \tvoid dispose() {\n+\t\tActivator.trace(\"(GitDocument) dispose: \" + resource);\n \t\tdoc2repo.remove(this);\n \t\tRepository repository = getRepository();\n \t\tif (repository != null)\ndiff --git a/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/decorators/GitQuickDiffProvider.java b/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/decorators/GitQuickDiffProvider.java\nindex 6c71f3c..91efb56 100644\n--- a/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/decorators/GitQuickDiffProvider.java\n+++ b/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/decorators/GitQuickDiffProvider.java\n@@ -42,6 +42,7 @@\n \t * Create the GitQuickDiffProvider instance\n \t */\n \tpublic GitQuickDiffProvider() {\n+\t\tActivator.trace(\"(GitQuickDiffProvider) constructor\");\n \t\t// Empty\n \t}\n \n@@ -77,6 +78,7 @@ public boolean isEnabled() {\n \t}\n \n \tpublic void setActiveEditor(ITextEditor editor) {\n+\t\tActivator.trace(\"(GitQuickDiffProvider) setActiveEditor: \" + editor.getTitle());\n \t\tIEditorInput editorInput = editor.getEditorInput();\n \t\tresource = ResourceUtil.getResource(editorInput);\n \t}\n-- \n1.6.2.1.345.g89fb\n"},{"id":"110235","messageId":"1238697991-12990-6-git-send-email-robin.rosenberg@dewire.com","threadId":"18696","inReplyTo":"1238697991-12990-5-git-send-email-robin.rosenberg@dewire.com","subject":"[EGIT PATCH 5/5] Cache resolved ids in quickdiff document for faster update","fromName":"Robin Rosenberg","fromEmail":"robin.rosenberg@dewire.com","sentAt":"2009-04-02T18:46:31Z","receivedAt":"2009-04-02T18:46:31Z","isPatch":true,"sender":{"key":"robin.rosenberg@dewire.com","avatar":"https://avatars.githubusercontent.com/u/46357?v=4"},"body":"We do this by caching the commit, tree and blob ids and can then\nvery quickly decide whether a change in baseline actually results in a\nchanged version of the reference blob used for quickdiff.\n\nSigned-off-by: Robin Rosenberg \n---\n .../egit/ui/internal/decorators/GitDocument.java | 70 +++++++++++++++++---\n 1 files changed, 60 insertions(+), 10 deletions(-)\n\ndiff --git a/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/decorators/GitDocument.java b/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/decorators/GitDocument.java\nindex 347e6fc..59a738c 100644\n--- a/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/decorators/GitDocument.java\n+++ b/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/decorators/GitDocument.java\n@@ -20,8 +20,11 @@\n import org.spearce.egit.core.GitProvider;\n import org.spearce.egit.core.project.RepositoryMapping;\n import org.spearce.egit.ui.Activator;\n+import org.spearce.jgit.lib.AnyObjectId;\n+import org.spearce.jgit.lib.Commit;\n import org.spearce.jgit.lib.Constants;\n import org.spearce.jgit.lib.IndexChangedEvent;\n+import org.spearce.jgit.lib.ObjectId;\n import org.spearce.jgit.lib.ObjectLoader;\n import org.spearce.jgit.lib.RefsChangedEvent;\n import org.spearce.jgit.lib.Repository;\n@@ -31,6 +34,11 @@\n \n class GitDocument extends Document implements RepositoryListener {\n \tprivate final IResource resource;\n+\n+\tprivate AnyObjectId lastCommit;\n+\tprivate AnyObjectId lastTree;\n+\tprivate AnyObjectId lastBlob;\n+\n \tstatic Map doc2repo = new WeakHashMap();\n \n \tstatic GitDocument create(final IResource resource) throws IOException {\n@@ -51,29 +59,68 @@ private GitDocument(IResource resource) {\n \t\tGitDocument.doc2repo.put(this, getRepository());\n \t}\n \n+\tprivate void setResolved(final AnyObjectId commit, final AnyObjectId tree, final AnyObjectId blob, final String value) {\n+\t\tlastCommit = commit != null ? commit.copy() : null;\n+\t\tlastTree = tree != null ? tree.copy() : null;\n+\t\tlastBlob = blob != null ? blob.copy() : null;\n+\t\tset(value);\n+\t\tif (blob != null)\n+\t\t\tActivator.trace(\"(GitDocument) resolved \" + resource + \" to \" + lastBlob + \" in \" + lastCommit + \"/\" + lastTree);\n+\t\telse\n+\t\t\tActivator.trace(\"(GitDocument) unresolved \" + resource);\n+\t}\n+\n \tvoid populate() throws IOException {\n \t\tActivator.trace(\"(GitDocument) populate: \" + resource);\n-\t\tset(\"\");\n \t\tfinal IProject project = resource.getProject();\n \t\tRepositoryMapping mapping = RepositoryMapping.getMapping(project);\n-\t\tif (mapping == null)\n+\t\tif (mapping == null) {\n+\t\t\tsetResolved(null, null, null, \"\");\n \t\t\treturn;\n+\t\t}\n \t\tfinal String gitPath = mapping.getRepoRelativePath(resource);\n-\t\tfinal Repository repository = getRepository();\n+\t\tfinal Repository repository = mapping.getRepository();\n \t\tString baseline = GitQuickDiffProvider.baseline.get(repository);\n \t\tif (baseline == null)\n \t\t\tbaseline = Constants.HEAD;\n-\t\tTree baselineTree = repository.mapTree(baseline);\n-\t\tif (baselineTree == null) {\n+\t\tObjectId commitId = repository.resolve(baseline);\n+\t\tif (commitId != null) {\n+\t\t\tif (commitId.equals(lastCommit)) {\n+\t\t\t\tActivator.trace(\"(GitDocument) already resolved\"); \n+\t\t\t\treturn;\n+\t\t\t}\n+\t\t} else {\n \t\t\tActivator.logError(\"Could not resolve quickdiff baseline \"\n \t\t\t\t\t+ baseline + \" corresponding to \" + resource + \" in \"\n \t\t\t\t\t+ repository, new Throwable());\n+\t\t\tsetResolved(null, null, null, \"\");\n+\t\t\treturn;\n+\t\t}\n+\t\tCommit baselineCommit = repository.mapCommit(commitId);\n+\t\tif (baselineCommit == null) {\n+\t\t\tActivator.logError(\"Could not load commit \" + commitId + \" for \"\n+\t\t\t\t\t+ baseline + \" corresponding to \" + resource + \" in \"\n+\t\t\t\t\t+ repository, new Throwable());\n+\t\t\tsetResolved(null, null, null, \"\");\n+\t\t\treturn;\n+\t\t}\n+\t\tObjectId treeId = baselineCommit.getTreeId();\n+\t\tif (treeId.equals(lastTree)) {\n+\t\t\tActivator.trace(\"(GitDocument) already resolved\"); \n+\t\t\treturn;\n+\t\t}\n+\t\tTree baselineTree = baselineCommit.getTree();\n+\t\tif (baselineTree == null) {\n+\t\t\tActivator.logError(\"Could not load tree \" + treeId + \" for \"\n+\t\t\t\t\t+ baseline + \" corresponding to \" + resource + \" in \"\n+\t\t\t\t\t+ repository, new Throwable());\n+\t\t\tsetResolved(null, null, null, \"\");\n \t\t\treturn;\n \t\t}\n-\t\tTreeEntry blobEnry = baselineTree.findBlobMember(gitPath);\n-\t\tif (blobEnry != null) {\n+\t\tTreeEntry blobEntry = baselineTree.findBlobMember(gitPath);\n+\t\tif (blobEntry != null && !blobEntry.getId().equals(lastBlob)) {\n \t\t\tActivator.trace(\"(GitDocument) compareTo: \" + baseline);\n-\t\t\tObjectLoader loader = repository.openBlob(blobEnry.getId());\n+\t\t\tObjectLoader loader = repository.openBlob(blobEntry.getId());\n \t\t\tbyte[] bytes = loader.getBytes();\n \t\t\tString charset;\n \t\t\t// Get the encoding for the current version. As a matter of\n@@ -92,10 +139,13 @@ void populate() throws IOException {\n \t\t\t// Finally we could consider validating the content with respect\n \t\t\t// to the content. We don't do that here.\n \t\t\tString s = new String(bytes, charset);\n-\t\t\tset(s);\n+\t\t\tsetResolved(commitId, baselineTree.getId(), blobEntry.getId(), s);\n \t\t\tActivator.trace(\"(GitDocument) has reference doc, size=\" + s.length() + \" bytes\");\n \t\t} else {\n-\t\t\tActivator.trace(\"(GitDocument) no revision.\");\n+\t\t\tif (blobEntry == null)\n+\t\t\t\tsetResolved(null, null, null, \"\");\n+\t\t\telse\n+\t\t\t\tActivator.trace(\"(GitDocument) already resolved\"); \n \t\t}\n \t}\n \n-- \n1.6.2.1.345.g89fb\n"},{"id":"110448","messageId":"20090405202400.GO23521@spearce.org","threadId":"18696","inReplyTo":"1238697991-12990-3-git-send-email-robin.rosenberg@dewire.com","subject":"Re: [EGIT PATCH 2/5] quickdiff - Don't add GitDocument as repository listener more than once","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2009-04-05T20:24:00Z","receivedAt":"2009-04-05T20:24:00Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Robin Rosenberg wrote:\n> Doing so was very costly and led to sessions being locked\n> for a long time while refreshing the reference document used\n> by Eclipse's quickdiff feature.\n...\n> --- a/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java\n> +++ b/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java\n> @@ -1132,6 +1132,7 @@ public File getWorkDir() {\n> \t * @param l\n> \t */\n> \tpublic void addRepositoryChangedListener(final RepositoryListener l) {\n> +\t\tassert !listeners.contains(l);\n> \t\tlisteners.add(l);\n> \t}\n> \n> @@ -1150,6 +1151,7 @@ public void removeRepositoryChangedListener(final RepositoryListener l) {\n> \t * @param l\n> \t */\n> \tpublic static void addAnyRepositoryChangedListener(final RepositoryListener l) {\n> +\t\tassert !allListeners.contains(l);\n> \t\tallListeners.add(l);\n> \t}\n\nThat's a race condition. The two collections are Vectors so another\nthread can add the listener between your assertion point and the\nadd call.\n\nAlso, if this really is considered to be very bad behavior on the\npart of the application, maybe these should be real tests with\nexceptions thrown, so they can't be disabled when assertions are\nturned off in the JRE?\n\n-- \nShawn.\n"},{"id":"110450","messageId":"20090405203604.GP23521@spearce.org","threadId":"18696","inReplyTo":"1238697991-12990-6-git-send-email-robin.rosenberg@dewire.com","subject":"Re: [EGIT PATCH 5/5] Cache resolved ids in quickdiff document for faster update","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2009-04-05T20:36:04Z","receivedAt":"2009-04-05T20:36:04Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Robin Rosenberg wrote:\n> We do this by caching the commit, tree and blob ids and can then\n> very quickly decide whether a change in baseline actually results in a\n> changed version of the reference blob used for quickdiff.\n...\n> @@ -31,6 +34,11 @@\n> \n> class GitDocument extends Document implements RepositoryListener {\n> \tprivate final IResource resource;\n> +\n> +\tprivate AnyObjectId lastCommit;\n> +\tprivate AnyObjectId lastTree;\n> +\tprivate AnyObjectId lastBlob;\n\nShould have been \"ObjectId\"; I amended the patch.\n\n> +\t\tCommit baselineCommit = repository.mapCommit(commitId);\n> +\t\tTree baselineTree = baselineCommit.getTree();\n> +\t\tTreeEntry blobEntry = baselineTree.findBlobMember(gitPath);\n\nArrrgh. We're still using Commit/Tree/TreeEntry to read file paths?\n\nI'm applying this as-is, but we really need to start to transition\naway from them. I wanted to start deleting the mapCommit and its\nfriends from the Repository class.\n\n-- \nShawn.\n"},{"id":"110468","messageId":"200904052340.01864.robin.rosenberg.lists@dewire.com","threadId":"18696","inReplyTo":"20090405203604.GP23521@spearce.org","subject":"Re: [EGIT PATCH 5/5] Cache resolved ids in quickdiff document for faster update","fromName":"Robin Rosenberg","fromEmail":"robin.rosenberg.lists@dewire.com","sentAt":"2009-04-05T21:40:01Z","receivedAt":"2009-04-05T21:40:01Z","isPatch":true,"sender":{"key":"robin.rosenberg@dewire.com","avatar":"https://avatars.githubusercontent.com/u/46357?v=4"},"body":"söndag 05 april 2009 22:36:04 skrev \"Shawn O. Pearce\" :\n> Arrrgh. We're still using Commit/Tree/TreeEntry to read file paths?\n>\n> I'm applying this as-is, but we really need to start to transition\n> away from them. I wanted to start deleting the mapCommit and its\n> friends from the Repository class.\n\nYeah, but the new API is more awkward and error-prone to use. The old\nAPI is quite straightforward. I will try harder next time.\n\n-- robin\n"},{"id":"110469","messageId":"200904052341.15590.robin.rosenberg.lists@dewire.com","threadId":"18696","inReplyTo":"20090405202400.GO23521@spearce.org","subject":"Re: [EGIT PATCH 2/5] quickdiff - Don't add GitDocument as repository listener more than once","fromName":"Robin Rosenberg","fromEmail":"robin.rosenberg.lists@dewire.com","sentAt":"2009-04-05T21:41:15Z","receivedAt":"2009-04-05T21:41:15Z","isPatch":true,"sender":{"key":"robin.rosenberg@dewire.com","avatar":"https://avatars.githubusercontent.com/u/46357?v=4"},"body":"söndag 05 april 2009 22:24:00 skrev \"Shawn O. Pearce\" :\n> Robin Rosenberg wrote:\n> > Doing so was very costly and led to sessions being locked\n> > for a long time while refreshing the reference document used\n> > by Eclipse's quickdiff feature.\n> ...\n> > --- a/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java\n> > +++ b/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java\n> > @@ -1132,6 +1132,7 @@ public File getWorkDir() {\n> > \t * @param l\n> > \t */\n> > \tpublic void addRepositoryChangedListener(final RepositoryListener l) {\n> > +\t\tassert !listeners.contains(l);\n> > \t\tlisteners.add(l);\n> > \t}\n> > \n> > @@ -1150,6 +1151,7 @@ public void removeRepositoryChangedListener(final RepositoryListener l) {\n> > \t * @param l\n> > \t */\n> > \tpublic static void addAnyRepositoryChangedListener(final RepositoryListener l) {\n> > +\t\tassert !allListeners.contains(l);\n> > \t\tallListeners.add(l);\n> > \t}\n> \n> That's a race condition. The two collections are Vectors so another\n> thread can add the listener between your assertion point and the\n> add call.\n\nThat would be another programming error, to add the same listener from different threads. Hopefully it would be caught on the next run. Adding synchronized might be too much since I cannot disable it by disabling assert.\n\n> Also, if this really is considered to be very bad behavior on the\n> part of the application, maybe these should be real tests with\n> exceptions thrown, so they can't be disabled when assertions are\n> turned off in the JRE?\n\nIt's usually not terribly bad, but what happens isn't well defined. If it happens it's a programming error. Especially now that the API bans it.\n\nAsserts are for detecting programming errors during program test. If the tests are good enough there will never be a reason for this assert to fail and so it won't be necessary in deployed code. We could probably have lots more of these\ndeclarations of assumptions, most of which would be less obvious than this simple test.\n\nIn particular asserts are for detecting bad conditions the programmer controls (or should control). Anything that can be the result of user actions or depending on the runtime environment should have non assert tests.\n\n-- robin\n"},{"id":"110471","messageId":"20090405214332.GC23604@spearce.org","threadId":"18696","inReplyTo":"200904052340.01864.robin.rosenberg.lists@dewire.com","subject":"Re: [EGIT PATCH 5/5] Cache resolved ids in quickdiff document for faster update","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2009-04-05T21:43:32Z","receivedAt":"2009-04-05T21:43:32Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Robin Rosenberg wrote:\n> s?ndag 05 april 2009 22:36:04 skrev \"Shawn O. Pearce\" :\n> > Arrrgh. We're still using Commit/Tree/TreeEntry to read file paths?\n> >\n> > I'm applying this as-is, but we really need to start to transition\n> > away from them. I wanted to start deleting the mapCommit and its\n> > friends from the Repository class.\n> \n> Yeah, but the new API is more awkward and error-prone to use. The old\n> API is quite straightforward.\n\nI see. Then I failed.\n\nWhat can we do to reduce that?\n\n> I will try harder next time.\n\nShouldn't be necessary. :-)\n\n-- \nShawn.\n"}]}