Use String.intern() instead of own StringPool

bundles/org.eclipse.equinox.preferences/src/org/eclipse/core/internal/preferences/EclipsePreferences.java

@@ -23,6 +23,7 @@
 import java.nio.file.Path;
 import java.nio.file.StandardCopyOption;
 import java.util.*;
+import java.util.concurrent.ConcurrentHashMap;
 import org.eclipse.core.internal.runtime.RuntimeLog;
 import org.eclipse.core.runtime.*;
 import org.eclipse.core.runtime.preferences.*;
@@ -261,7 +262,6 @@ protected static void convertFromProperties(EclipsePreferences node, Properties
 					childNode.firePreferenceEvent(key, oldValue, value);
 			}
 		}
-		PreferencesService.getDefault().shareStrings();
 	}
 
 	private final Object writeLock = new Object();
@@ -387,9 +387,9 @@ public void flush() throws BackingStoreException {
 			toFlush = internalFlush();
 		}
 		// if we aren't at the right level, then flush the appropriate node
-		if (toFlush != null)
+		if (toFlush != null) {
 			toFlush.flush();
-		PreferencesService.getDefault().shareStrings();
+		}
 	}
 
 	/*
@@ -629,15 +629,23 @@ protected String internalPut(String key, String newValue) {
 			// illegal state if this node has been removed
 			checkRemoved();
 			String oldValue = properties.get(key);
-			if (oldValue != null && oldValue.equals(newValue))
+			if (oldValue != null && oldValue.equals(newValue)) {
 				return oldValue;
-			if (DEBUG_PREFERENCE_SET)
+			} else if (DEBUG_PREFERENCE_SET) {
 				PrefsMessages.message("Setting preference: " + absolutePath() + '/' + key + '=' + newValue); //$NON-NLS-1$
-			properties = properties.put(key, newValue);
+			}
+			properties = properties.put(intern(key), intern(newValue));
 			return oldValue;
 		}
 	}
 
+	private static final Map<String, String> STRING_POOL = new ConcurrentHashMap<>();
+
+	private static String intern(String s) {
+		String cached = STRING_POOL.putIfAbsent(s, s);
+		return cached != null ? cached : s;
+	}
+
 	/*
 	 * Subclasses to over-ride.
 	 */
@@ -1039,28 +1047,6 @@ protected void save(IPath location) throws BackingStoreException {
 		write(table, location);
 	}
 
-	/**
-	 * Traverses the preference hierarchy rooted at this node, and adds all
-	 * preference key and value strings to the provided pool. If an added string was
-	 * already in the pool, all references will be replaced with the canonical copy
-	 * of the string.
-	 *
-	 * @param pool The pool to share strings in
-	 */
-	public void shareStrings(StringPool pool) {
-		// thread safety: copy reference in case of concurrent change
-		ImmutableMap temp;
-		synchronized (childAndPropertyLock) {
-			temp = properties;
-		}
-		temp.shareStrings(pool);
-		for (IEclipsePreferences child : getChildren(false)) {
-			if (child instanceof EclipsePreferences) {
-				((EclipsePreferences) child).shareStrings(pool);
-			}
-		}
-	}
-
 	/*
 	 * Encode the given path and key combo to a form which is suitable for
 	 * persisting or using when searching. If the key contains a slash character

bundles/org.eclipse.equinox.preferences/src/org/eclipse/core/internal/preferences/ImmutableMap.java

@@ -35,10 +35,10 @@ static class ArrayMap extends ImmutableMap {
 		/**
 		 * The table keys
 		 */
-		private String[] keyTable;
+		private final String[] keyTable;
 
-		private int threshold;
-		private String[] valueTable;
+		private final int threshold;
+		private final String[] valueTable;
 
 		ArrayMap(int size) {
 			this.elementSize = 0;
@@ -147,27 +147,6 @@ public ImmutableMap removeKey(String key) {
 			return this;
 		}
 
-		@Override
-		public void shareStrings(StringPool set) {
-			// copy elements for thread safety
-			String[] array = keyTable;
-			if (array == null)
-				return;
-			for (int i = 0; i < array.length; i++) {
-				String o = array[i];
-				if (o != null)
-					array[i] = set.add(o);
-			}
-			array = valueTable;
-			if (array == null)
-				return;
-			for (int i = 0; i < array.length; i++) {
-				String o = array[i];
-				if (o != null)
-					array[i] = set.add(o);
-			}
-		}
-
 		@Override
 		public int size() {
 			return elementSize;
@@ -257,10 +236,6 @@ protected static ImmutableMap createMap(int i) {
 	 */
 	public abstract ImmutableMap removeKey(String key);
 
-	public void shareStrings(StringPool set) {
-		// nothing to do
-	}
-
 	/**
 	 * Returns the number of keys in this map.
 	 * 

bundles/org.eclipse.equinox.preferences/src/org/eclipse/core/internal/preferences/PreferencesService.java

@@ -33,10 +33,6 @@
  * @since 3.0
  */
 public class PreferencesService implements IPreferencesService {
-	/**
-	 * The interval between passes over the preference tree to canonicalize strings.
-	 */
-	private static final long STRING_SHARING_INTERVAL = 300000;
 	private static final String MATCH_TYPE_PREFIX = "prefix"; //$NON-NLS-1$
 
 	// the order of search scopes when people don't have a specific order set
@@ -57,11 +53,6 @@ public class PreferencesService implements IPreferencesService {
 	private Object registryHelper = null;
 	private final Map<String, EclipsePreferences> defaultScopes = new HashMap<>();
 
-	/**
-	 * The last time analysis was done to remove duplicate strings
-	 */
-	private long lastStringSharing = 0;
-
 	/*
 	 * Create and return an IStatus object with ERROR severity and the given message
 	 * and exception.
@@ -100,11 +91,6 @@ public void applyPreferences(IEclipsePreferences tree, IPreferenceFilter[] filte
 			} catch (BackingStoreException e) {
 				throw new CoreException(createStatusError(PrefsMessages.preferences_saveProblems, e));
 			}
-
-			// this typically causes a major change to the preference tree, so force string
-			// sharing
-			lastStringSharing = 0;
-			shareStrings();
 		} catch (BackingStoreException e) {
 			throw new CoreException(createStatusError(PrefsMessages.preferences_applyProblems, e));
 		}
@@ -211,13 +197,9 @@ public boolean visit(IEclipsePreferences node) throws BackingStoreException {
 			throw new CoreException(createStatusError(PrefsMessages.preferences_saveProblems, e));
 		}
 
-		if (EclipsePreferences.DEBUG_PREFERENCE_GENERAL)
-			PrefsMessages.message(
-					"Current list of all settings: " + ((EclipsePreferences) getRootNode()).toDeepDebugString()); //$NON-NLS-1$
-		// this typically causes a major change to the preference tree, so force string
-		// sharing
-		lastStringSharing = 0;
-		shareStrings();
+		if (EclipsePreferences.DEBUG_PREFERENCE_GENERAL) {
+			PrefsMessages.message("Current list of all settings: " + ((EclipsePreferences) getRootNode()).toDeepDebugString()); //$NON-NLS-1$
+		}
 		return result;
 	}
 
@@ -959,20 +941,6 @@ public void setRegistryHelper(Object registryHelper) {
 		this.registryHelper = registryHelper;
 	}
 
-	/**
-	 * Shares all duplicate equal strings referenced by the preference service.
-	 */
-	void shareStrings() {
-		long now = System.currentTimeMillis();
-		if (now - lastStringSharing < STRING_SHARING_INTERVAL)
-			return;
-		StringPool pool = new StringPool();
-		root.shareStrings(pool);
-		if (EclipsePreferences.DEBUG_PREFERENCE_GENERAL)
-			System.out.println("Preference string sharing saved: " + pool.getSavedStringCount()); //$NON-NLS-1$
-		lastStringSharing = now;
-	}
-
 	/*
 	 * Return a tree which contains only nodes and keys which are applicable to the
 	 * given filter.