From ed829cf0c68371ba77ec35417e215cb7b3bb78c7 Mon Sep 17 00:00:00 2001 From: Thomas Wolf Date: Mon, 10 Jul 2017 10:43:10 +0200 Subject: [PATCH] Reduce allocations in decorator The GitLightweightDecorator created a new DecorationHelper for each object to be decorated. That's not necessary; the DecorationHelper is stateless. Use a singleton instance instead. Also use a singleton instance ResourceState for ignored resources. For such resources: * we're not interested in any of the other properties, * the default values of the other properties are fine. While these allocations are not the cause of the slowness of updating decorations, they add unnecessary overhead, memory usage, and pressure on the garbage collector. Bug: 500106 Change-Id: If80e60029c6b9f821ea4ff6f18d4cd95835fb96b Signed-off-by: Thomas Wolf --- .../decorators/DecoratableResourceAdapter.java | 8 ++-- .../decorators/GitLightweightDecorator.java | 8 ++-- .../egit/ui/internal/resources/ResourceState.java | 3 +- .../internal/resources/ResourceStateFactory.java | 54 +++++++++++++++------- 4 files changed, 46 insertions(+), 27 deletions(-) diff --git a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/decorators/DecoratableResourceAdapter.java b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/decorators/DecoratableResourceAdapter.java index d623547a6..6efa16cba 100644 --- a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/decorators/DecoratableResourceAdapter.java +++ b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/decorators/DecoratableResourceAdapter.java @@ -44,7 +44,7 @@ class DecoratableResourceAdapter extends DecoratableResource { GitTraceLocation.getTrace().trace( GitTraceLocation.DECORATION.getLocation(), "Decorate " + resourceToWrap.getFullPath()); //$NON-NLS-1$ - start = System.currentTimeMillis(); + start = System.nanoTime(); } try { RepositoryMapping mapping = RepositoryMapping @@ -65,7 +65,7 @@ class DecoratableResourceAdapter extends DecoratableResource { setConflicts(baseState.hasConflicts()); setAssumeUnchanged(baseState.isAssumeUnchanged()); setStagingState(baseState.getStagingState()); - if (isRepositoryContainer()) { + if (isRepositoryContainer() && !isIgnored()) { // We only need this very expensive info for for decorating // projects and folders that are submodule or nested repository // roots @@ -84,8 +84,8 @@ class DecoratableResourceAdapter extends DecoratableResource { GitTraceLocation .getTrace() .trace(GitTraceLocation.DECORATION.getLocation(), - "Decoration took " + (System.currentTimeMillis() - start) //$NON-NLS-1$ - + " ms"); //$NON-NLS-1$ + "Decoration took " + (System.nanoTime() - start) //$NON-NLS-1$ + + " ns"); //$NON-NLS-1$ } } diff --git a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/decorators/GitLightweightDecorator.java b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/decorators/GitLightweightDecorator.java index 4bf63e28b..4cddc72c1 100644 --- a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/decorators/GitLightweightDecorator.java +++ b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/decorators/GitLightweightDecorator.java @@ -113,6 +113,9 @@ public class GitLightweightDecorator extends LabelProvider implements private static RGB defaultBackgroundRgb; + private final DecorationHelper helper = new DecorationHelper( + Activator.getDefault().getPreferenceStore()); + private RepositoryMappingChangeListener mappingChangeListener = new RepositoryMappingChangeListener() { @Override @@ -232,8 +235,6 @@ public class GitLightweightDecorator extends LabelProvider implements return; } IDecoratableResource decoratableResource = null; - final DecorationHelper helper = new DecorationHelper( - Activator.getDefault().getPreferenceStore()); try { decoratableResource = new DecoratableResourceAdapter(indexDiffData, resource); } catch (IOException e) { @@ -281,9 +282,6 @@ public class GitLightweightDecorator extends LabelProvider implements return; } - final DecorationHelper helper = new DecorationHelper( - Activator.getDefault().getPreferenceStore()); - helper.decorate(decoration, decoRes); } diff --git a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/resources/ResourceState.java b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/resources/ResourceState.java index a237ec39f..5ddf813f9 100644 --- a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/resources/ResourceState.java +++ b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/resources/ResourceState.java @@ -95,7 +95,8 @@ public class ResourceState implements IResourceState { @Override public final boolean hasUnstagedChanges() { - return !isTracked() || isDirty() || isMissing() || hasConflicts(); + return !isIgnored() + && (!isTracked() || isDirty() || isMissing() || hasConflicts()); } /** diff --git a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/resources/ResourceStateFactory.java b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/resources/ResourceStateFactory.java index 33dd069f2..d65b477f4 100644 --- a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/resources/ResourceStateFactory.java +++ b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/resources/ResourceStateFactory.java @@ -59,6 +59,18 @@ public class ResourceStateFactory { @NonNull public static final IResourceState UNKNOWN_STATE = new ResourceState(); + /** + * Singleton {@link IResourceState} returned for ignored files. + */ + @NonNull + private static final IResourceState IGNORED = new ResourceState() { + + @Override + public boolean isIgnored() { + return true; + } + }; + @NonNull private static final ResourceStateFactory INSTANCE = new ResourceStateFactory(); @@ -230,7 +242,6 @@ public class ResourceStateFactory { // Could not be made relative. return UNKNOWN_STATE; } - ResourceState result = new ResourceState(); if (file.isContainer()) { if (!repoRelativePath.endsWith("/")) { //$NON-NLS-1$ repoRelativePath += '/'; @@ -239,25 +250,29 @@ public class ResourceStateFactory { // The Eclipse resource model handles a symlink to a folder like // the container it refers to but git status handles the symlink // source like a special file. - extractFileProperties(indexDiffData, repoRelativePath, result); + return extractFileProperties(indexDiffData, repoRelativePath); } else { - extractContainerProperties(indexDiffData, repoRelativePath, - file, result); + return extractContainerProperties(indexDiffData, + repoRelativePath, file); } } else { - extractFileProperties(indexDiffData, repoRelativePath, result); + return extractFileProperties(indexDiffData, repoRelativePath); } - return result; } - private void extractFileProperties(@NonNull IndexDiffData indexDiffData, - @NonNull String repoRelativePath, @NonNull ResourceState state) { + private @NonNull IResourceState extractFileProperties( + @NonNull IndexDiffData indexDiffData, + @NonNull String repoRelativePath) { Set ignoredFiles = indexDiffData.getIgnoredNotInIndex(); boolean ignored = ignoredFiles.contains(repoRelativePath) || containsPrefixPath(ignoredFiles, repoRelativePath); - state.setIgnored(ignored); + if (ignored) { + // Leave the rest at the default (false, NOT_STAGED) + return IGNORED; + } + ResourceState state = new ResourceState(); Set untracked = indexDiffData.getUntracked(); - state.setTracked(!ignored && !untracked.contains(repoRelativePath)); + state.setTracked(!untracked.contains(repoRelativePath)); Set added = indexDiffData.getAdded(); Set removed = indexDiffData.getRemoved(); @@ -286,19 +301,23 @@ public class ResourceStateFactory { Set assumeUnchanged = indexDiffData.getAssumeUnchanged(); state.setAssumeUnchanged(assumeUnchanged.contains(repoRelativePath)); + return state; } - private void extractContainerProperties( + private @NonNull IResourceState extractContainerProperties( @NonNull IndexDiffData indexDiffData, - @NonNull String repoRelativePath, @NonNull FileSystemItem directory, - @NonNull ResourceState state) { + @NonNull String repoRelativePath, + @NonNull FileSystemItem directory) { Set ignoredFiles = indexDiffData.getIgnoredNotInIndex(); - Set untrackedFolders = indexDiffData.getUntrackedFolders(); boolean ignored = containsPrefixPath(ignoredFiles, repoRelativePath) || !directory.hasContainerAnyFiles(); - state.setIgnored(ignored); - state.setTracked(!ignored - && !containsPrefixPath(untrackedFolders, repoRelativePath)); + if (ignored) { + return IGNORED; + } + ResourceState state = new ResourceState(); + Set untrackedFolders = indexDiffData.getUntrackedFolders(); + state.setTracked( + !containsPrefixPath(untrackedFolders, repoRelativePath)); // containers are marked as staged whenever file was added, removed or // changed @@ -321,6 +340,7 @@ public class ResourceStateFactory { state.setDirty(containsPrefix(modified, repoRelativePath) || containsPrefix(untracked, repoRelativePath) || containsPrefix(missing, repoRelativePath)); + return state; } private boolean containsPrefix(Set collection, String prefix) { -- 2.11.4.GIT