Bug 570896: Refactor URI comparisons
The functionality added to URIUtil would become public API if left in
the current place, so move it to an alternate class in an internal
package to avoid public API growth.
Change-Id: Ibccc01f4f04177090fbc9ce91333ff755405d587
Signed-off-by: Alex Blewitt <alex.blewitt@gmail.com>
diff --git a/bundles/org.eclipse.core.filesystem/src/org/eclipse/core/filesystem/IFileStore.java b/bundles/org.eclipse.core.filesystem/src/org/eclipse/core/filesystem/IFileStore.java
index b90cfa7..15f3397 100644
--- a/bundles/org.eclipse.core.filesystem/src/org/eclipse/core/filesystem/IFileStore.java
+++ b/bundles/org.eclipse.core.filesystem/src/org/eclipse/core/filesystem/IFileStore.java
@@ -19,6 +19,7 @@
import java.io.OutputStream;
import java.net.URI;
import org.eclipse.core.filesystem.provider.FileStore;
+import org.eclipse.core.internal.filesystem.FileStoreUtil;
import org.eclipse.core.runtime.*;
/**
@@ -572,10 +573,13 @@
* be adjacent. This is equivalent to the natural order on the path strings,
* with the extra condition that the path separator is ordered before all other
* characters. (Ex: "/foo" < "/foo/zzz" < "/fooaaa").
- *
+ *
* @since org.eclipse.core.filesystem 1.9
*/
public default int compareTo(IFileStore other) {
+ int compare = FileStoreUtil.compareStringOrNull(this.getFileSystem().getScheme(), other.getFileSystem().getScheme());
+ if (compare != 0)
+ return compare;
// compare based on URI path segment values
URI uri1;
URI uri2;
@@ -591,7 +595,7 @@
// protect against misbehaving 3rd party code in file system implementations
uri2 = null;
}
- // use old slow compare for compatibility reason. Does have a memory hotspot see bug 570896
- return URIUtil.comparePathUri(uri1, uri2);
+ // use old slow compare for compatibility reason. Does have a memory hotspot see bug 570896
+ return FileStoreUtil.comparePathUri(uri1, uri2);
}
-}
\ No newline at end of file
+}
diff --git a/bundles/org.eclipse.core.filesystem/src/org/eclipse/core/filesystem/URIUtil.java b/bundles/org.eclipse.core.filesystem/src/org/eclipse/core/filesystem/URIUtil.java
index d138894..bf0f55e 100644
--- a/bundles/org.eclipse.core.filesystem/src/org/eclipse/core/filesystem/URIUtil.java
+++ b/bundles/org.eclipse.core.filesystem/src/org/eclipse/core/filesystem/URIUtil.java
@@ -175,150 +175,6 @@
}
/**
- * Compares URIs by normalized IPath
- * This is the old slow implementation and has a memory hotspot see bug 570896. Prefer to use compareNormalisedUri!
- * @since org.eclipse.core.filesystem 1.9
- */
- public static int comparePathUri(URI uri1, URI uri2) {
- if (uri1 == null && uri2 == null)
- return 0;
- int compare;
- // Fixed compare contract sgn(compare(x, y)) == -sgn(compare(y, x))
- // in case of Exceptions:
- if ((compare = nullsLast(uri1, uri2)) != 0)
- return compare;
- // compare hosts
- compare = compareStringOrNull(uri1.getHost(), uri2.getHost());
- if (compare != 0)
- return compare;
- // compare user infos
- compare = compareStringOrNull(uri1.getUserInfo(), uri2.getUserInfo());
- if (compare != 0)
- return compare;
- // compare ports
- int port1 = uri1.getPort();
- int port2 = uri2.getPort();
- if (port1 != port2)
- return port1 - port2;
-
- IPath path1 = new Path(uri1.getPath());
- IPath path2 = new Path(uri2.getPath());
- // compare devices
- compare = compareStringOrNull(path1.getDevice(), path2.getDevice());
- if (compare != 0)
- return compare;
- // compare segments
- int segmentCount1 = path1.segmentCount();
- int segmentCount2 = path2.segmentCount();
- for (int i = 0; (i < segmentCount1) && (i < segmentCount2); i++) {
- compare = path1.segment(i).compareTo(path2.segment(i));
- if (compare != 0)
- return compare;
- }
- //all segments are equal, so compare based on number of segments
- compare = segmentCount1 - segmentCount2;
- if (compare != 0)
- return compare;
- //same number of segments, so compare query
- return compareStringOrNull(uri1.getQuery(), uri2.getQuery());
- }
-
- /**
- * Compares already normalized URIs
- * This is a fast implementation without memory allocation (Bug 570896).
- * note: if pathes contain different segment count this is != uri1.compareNormalisedUri(uri2)
- * @since org.eclipse.core.filesystem 1.9
- */
- public static int compareNormalisedUri(URI uri1, URI uri2) {
- if (uri1 == null && uri2 == null)
- return 0;
- int c;
- if ((c = nullsLast(uri1, uri2)) != 0)
- return c;
- // avoid to use IPath here due to high ephemeral memory allocation (Bug 570896)
- if ((c = compareStringOrNull(uri1.getAuthority(), uri2.getAuthority())) != 0)
- return c;
- if ((c = compareStringOrNull(uri1.getScheme(), uri2.getScheme())) != 0)
- return c;
- if ((c = comparePathSegments(uri1.getPath(), uri2.getPath())) != 0)
- return c;
- if ((c = compareStringOrNull(uri1.getQuery(), uri2.getQuery())) != 0)
- return c;
- return c;
- }
-
- static int nullsLast(Object c1, Object c2) {
- if (c1 == null) {
- if (c2 == null)
- return 0;
- return 1;
- }
- if (c2 == null)
- return -1;
- return 0;
- }
-
- static int comparePathSegments(String p1, String p2) {
- int compare;
- compare = compareSlashFirst(p1, p2);
- if (compare != 0)
- return compare;
- // all segments are equal, so compare based on number of segments
- int segmentCount1 = countCharButNotAtEnd(p1, '/');
- int segmentCount2 = countCharButNotAtEnd(p2, '/');
- compare = segmentCount1 - segmentCount2;
- return compare;
- }
-
- static int compareSlashFirst(String value, String other) {
- int len1 = value.length();
- int len2 = other.length();
- int lim = Math.min(len1, len2);
- for (int k = 0; k < lim; k++) {
- char c1 = value.charAt(k);
- char c2 = other.charAt(k);
- if (c1 != c2) {
- // '/' first
- if (c1 == '/')
- return -1;
- if (c2 == '/')
- return 1;
- return c1 - c2;
- }
- }
- // ignore "/" at the end
- if (value.endsWith("/")) //$NON-NLS-1$
- len1 -= 1;
- if (other.endsWith("/")) //$NON-NLS-1$
- len2 -= 1;
- return len1 - len2;
- }
-
- static int countCharButNotAtEnd(String str, char c) {
- int count = 0;
- for (int i = 0; i < str.length() - 1; i++) {
- if (str.charAt(i) == c)
- count++;
- }
- return count;
- }
-
- /**
- * Compares two strings that are possibly null.
- * @since org.eclipse.core.filesystem 1.9
- */
- public static int compareStringOrNull(String string1, String string2) {
- if (string1 == null) {
- if (string2 == null)
- return 0;
- return 1;
- }
- if (string2 == null)
- return -1;
- return string1.compareTo(string2);
- }
-
- /**
* Private constructor to prevent instantiation.
*/
private URIUtil() {
diff --git a/bundles/org.eclipse.core.filesystem/src/org/eclipse/core/internal/filesystem/FileStoreUtil.java b/bundles/org.eclipse.core.filesystem/src/org/eclipse/core/internal/filesystem/FileStoreUtil.java
new file mode 100644
index 0000000..0dbc549
--- /dev/null
+++ b/bundles/org.eclipse.core.filesystem/src/org/eclipse/core/internal/filesystem/FileStoreUtil.java
@@ -0,0 +1,172 @@
+/*******************************************************************************
+ * Copyright (c) 2021 Joerg Kubitz and others.
+ *
+ * This program and the accompanying materials
+ * are made available under the terms of the Eclipse Public License 2.0
+ * which accompanies this distribution, and is available at
+ * https://www.eclipse.org/legal/epl-2.0/
+ *
+ * SPDX-License-Identifier: EPL-2.0
+ *
+ * Contributors:
+ * Joerg Kubitz - initial API and implementation
+ *******************************************************************************/
+package org.eclipse.core.internal.filesystem;
+
+import java.net.URI;
+import org.eclipse.core.runtime.IPath;
+import org.eclipse.core.runtime.Path;
+
+/**
+ * Provides internal utility functions for comparing FileStores and paths
+ */
+public final class FileStoreUtil {
+
+ private FileStoreUtil() {
+ // Not to be instantiated
+ }
+
+ /**
+ * Compares URIs by normalized IPath
+ * This is the old slow implementation and has a memory hotspot see bug 570896. Prefer to use compareNormalisedUri!
+ * @since org.eclipse.core.filesystem 1.9
+ */
+ public static int comparePathUri(URI uri1, URI uri2) {
+ if (uri1 == null && uri2 == null)
+ return 0;
+ int compare;
+ // Fixed compare contract sgn(compare(x, y)) == -sgn(compare(y, x))
+ // in case of Exceptions:
+ if ((compare = nullsLast(uri1, uri2)) != 0)
+ return compare;
+ // compare hosts
+ compare = compareStringOrNull(uri1.getHost(), uri2.getHost());
+ if (compare != 0)
+ return compare;
+ // compare user infos
+ compare = compareStringOrNull(uri1.getUserInfo(), uri2.getUserInfo());
+ if (compare != 0)
+ return compare;
+ // compare ports
+ int port1 = uri1.getPort();
+ int port2 = uri2.getPort();
+ if (port1 != port2)
+ return port1 - port2;
+
+ IPath path1 = new Path(uri1.getPath());
+ IPath path2 = new Path(uri2.getPath());
+ // compare devices
+ compare = compareStringOrNull(path1.getDevice(), path2.getDevice());
+ if (compare != 0)
+ return compare;
+ // compare segments
+ int segmentCount1 = path1.segmentCount();
+ int segmentCount2 = path2.segmentCount();
+ for (int i = 0; (i < segmentCount1) && (i < segmentCount2); i++) {
+ compare = path1.segment(i).compareTo(path2.segment(i));
+ if (compare != 0)
+ return compare;
+ }
+ //all segments are equal, so compare based on number of segments
+ compare = segmentCount1 - segmentCount2;
+ if (compare != 0)
+ return compare;
+ //same number of segments, so compare query
+ return compareStringOrNull(uri1.getQuery(), uri2.getQuery());
+ }
+
+ /**
+ * Compares already normalized URIs
+ * This is a fast implementation without memory allocation (Bug 570896).
+ * note: if pathes contain different segment count this is != uri1.compareNormalisedUri(uri2)
+ * @since org.eclipse.core.filesystem 1.9
+ */
+ public static int compareNormalisedUri(URI uri1, URI uri2) {
+ if (uri1 == null && uri2 == null)
+ return 0;
+ int c;
+ if ((c = nullsLast(uri1, uri2)) != 0)
+ return c;
+ // avoid to use IPath here due to high ephemeral memory allocation (Bug 570896)
+ if ((c = compareStringOrNull(uri1.getAuthority(), uri2.getAuthority())) != 0)
+ return c;
+ if ((c = compareStringOrNull(uri1.getScheme(), uri2.getScheme())) != 0)
+ return c;
+ if ((c = comparePathSegments(uri1.getPath(), uri2.getPath())) != 0)
+ return c;
+ if ((c = compareStringOrNull(uri1.getQuery(), uri2.getQuery())) != 0)
+ return c;
+ return c;
+ }
+
+ static int nullsLast(Object c1, Object c2) {
+ if (c1 == null) {
+ if (c2 == null)
+ return 0;
+ return 1;
+ }
+ if (c2 == null)
+ return -1;
+ return 0;
+ }
+
+ static int comparePathSegments(String p1, String p2) {
+ int compare;
+ compare = compareSlashFirst(p1, p2);
+ if (compare != 0)
+ return compare;
+ // all segments are equal, so compare based on number of segments
+ int segmentCount1 = countCharButNotAtEnd(p1, '/');
+ int segmentCount2 = countCharButNotAtEnd(p2, '/');
+ compare = segmentCount1 - segmentCount2;
+ return compare;
+ }
+
+ static int compareSlashFirst(String value, String other) {
+ int len1 = value.length();
+ int len2 = other.length();
+ int lim = Math.min(len1, len2);
+ for (int k = 0; k < lim; k++) {
+ char c1 = value.charAt(k);
+ char c2 = other.charAt(k);
+ if (c1 != c2) {
+ // '/' first
+ if (c1 == '/')
+ return -1;
+ if (c2 == '/')
+ return 1;
+ return c1 - c2;
+ }
+ }
+ // ignore "/" at the end
+ if (value.endsWith("/")) //$NON-NLS-1$
+ len1 -= 1;
+ if (other.endsWith("/")) //$NON-NLS-1$
+ len2 -= 1;
+ return len1 - len2;
+ }
+
+ static int countCharButNotAtEnd(String str, char c) {
+ int count = 0;
+ for (int i = 0; i < str.length() - 1; i++) {
+ if (str.charAt(i) == c)
+ count++;
+ }
+ return count;
+ }
+
+ /**
+ * Compares two strings that are possibly null.
+ * @since org.eclipse.core.filesystem 1.9
+ */
+ public static int compareStringOrNull(String string1, String string2) {
+ if (string1 == null) {
+ if (string2 == null)
+ return 0;
+ return 1;
+ }
+ if (string2 == null)
+ return -1;
+ return string1.compareTo(string2);
+ }
+}
diff --git a/bundles/org.eclipse.core.filesystem/src/org/eclipse/core/internal/filesystem/local/LocalFile.java b/bundles/org.eclipse.core.filesystem/src/org/eclipse/core/internal/filesystem/local/LocalFile.java
index 1f11c28..dc68df5 100644
--- a/bundles/org.eclipse.core.filesystem/src/org/eclipse/core/internal/filesystem/local/LocalFile.java
+++ b/bundles/org.eclipse.core.filesystem/src/org/eclipse/core/internal/filesystem/local/LocalFile.java
@@ -25,8 +25,7 @@
import org.eclipse.core.filesystem.URIUtil;
import org.eclipse.core.filesystem.provider.FileInfo;
import org.eclipse.core.filesystem.provider.FileStore;
-import org.eclipse.core.internal.filesystem.Messages;
-import org.eclipse.core.internal.filesystem.Policy;
+import org.eclipse.core.internal.filesystem.*;
import org.eclipse.core.runtime.*;
import org.eclipse.core.runtime.Path;
import org.eclipse.osgi.util.NLS;
@@ -472,7 +471,10 @@
@Override
public int compareTo(IFileStore other) {
+ int compare = FileStoreUtil.compareStringOrNull(this.getFileSystem().getScheme(), other.getFileSystem().getScheme());
+ if (compare != 0)
+ return compare;
// override with fast implementation:
- return URIUtil.compareNormalisedUri(this.toURI(), other.toURI());
+ return FileStoreUtil.compareNormalisedUri(this.toURI(), other.toURI());
}
}
diff --git a/bundles/org.eclipse.core.resources/src/org/eclipse/core/internal/resources/AliasManager.java b/bundles/org.eclipse.core.resources/src/org/eclipse/core/internal/resources/AliasManager.java
index 8c2ede7..5d0ebda 100644
--- a/bundles/org.eclipse.core.resources/src/org/eclipse/core/internal/resources/AliasManager.java
+++ b/bundles/org.eclipse.core.resources/src/org/eclipse/core/internal/resources/AliasManager.java
@@ -22,7 +22,6 @@
import java.util.*;
import org.eclipse.core.filesystem.EFS;
import org.eclipse.core.filesystem.IFileStore;
-import org.eclipse.core.filesystem.URIUtil;
import org.eclipse.core.internal.events.ILifecycleListener;
import org.eclipse.core.internal.events.LifecycleEvent;
import org.eclipse.core.internal.localstore.FileSystemResourceManager;
@@ -130,7 +129,7 @@
/**
* Map of FileStore->IResource OR FileStore->ArrayList of (IResource)
*/
- private final SortedMap<IFileStore, Object> map = new TreeMap<>(AliasManager::compareUri);
+ private final SortedMap<IFileStore, Object> map = new TreeMap<>(IFileStore::compareTo);
/**
* Adds the given resource to the map, keyed by the given location.
@@ -486,22 +485,6 @@
}
}
- /**
- * Returns the compare result when sorting the locations map. Comparison is
- * based on segments, so that paths with the most segments in common will always
- * be adjacent. This is equivalent to the natural order on the path strings,
- * with the extra condition that the path separator is ordered before all other
- * characters. (Ex: "/foo" < "/foo/zzz" < "/fooaaa").
- */
- static int compareUri(IFileStore store1, IFileStore store2) {
- // scheme takes precedence over all else
- int compare = URIUtil.compareStringOrNull(store1.getFileSystem().getScheme(),
- store2.getFileSystem().getScheme());
- if (compare != 0)
- return compare;
- return store1.compareTo(store2);
- }
-
@Override
public void handleEvent(LifecycleEvent event) {
/*
diff --git a/tests/org.eclipse.core.tests.resources/src/org/eclipse/core/tests/internal/alias/BasicAliasTest.java b/tests/org.eclipse.core.tests.resources/src/org/eclipse/core/tests/internal/alias/BasicAliasTest.java
index cebe393..5bead64 100644
--- a/tests/org.eclipse.core.tests.resources/src/org/eclipse/core/tests/internal/alias/BasicAliasTest.java
+++ b/tests/org.eclipse.core.tests.resources/src/org/eclipse/core/tests/internal/alias/BasicAliasTest.java
@@ -21,7 +21,7 @@
import java.util.stream.Collectors;
import org.eclipse.core.filesystem.EFS;
import org.eclipse.core.filesystem.IFileStore;
-import org.eclipse.core.filesystem.URIUtil;
+import org.eclipse.core.internal.filesystem.FileStoreUtil;
import org.eclipse.core.internal.resources.*;
import org.eclipse.core.resources.*;
import org.eclipse.core.runtime.*;
@@ -312,7 +312,7 @@
for (URI u1 : uriList) {
for (URI u2 : uriList) {
if (!u1.equals(u2)) {
- assertTrue("1.0", 0 != URIUtil.compareNormalisedUri(u1, u2));
+ assertTrue("1.0", 0 != FileStoreUtil.compareNormalisedUri(u1, u2));
}
}
}
@@ -398,7 +398,7 @@
}
}).collect(Collectors.toList());
// stable sort:
- List<URI> sorted = uriList.stream().sorted(URIUtil::compareNormalisedUri).collect(Collectors.toList());
+ List<URI> sorted = uriList.stream().sorted(FileStoreUtil::compareNormalisedUri).collect(Collectors.toList());
// proof sort order did not change
assertEquals("1.0", uriList, sorted);
}
