Skip to content

Commit

Permalink
EclipsePreferences: use simple HashMap
Browse files Browse the repository at this point in the history
Using a concurrent Map is a waste as every access is guarded by
childAndPropertyLock anyway.

+ add missing guards (was used unguarded in RootPreferences)
+ make getChild(String key, Object context, boolean create) atomic
  • Loading branch information
EcljpseB0T committed Aug 27, 2024
1 parent 4526f76 commit bd32d3e
Show file tree
Hide file tree
Showing 4 changed files with 36 additions and 32 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ Manifest-Version: 1.0
Bundle-ManifestVersion: 2
Bundle-Name: %pluginName
Bundle-SymbolicName: org.eclipse.equinox.preferences; singleton:=true
Bundle-Version: 3.11.100.qualifier
Bundle-Version: 3.11.200.qualifier
Bundle-Activator: org.eclipse.core.internal.preferences.Activator
Bundle-Vendor: %providerName
Bundle-Localization: plugin
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
import java.nio.file.*;
import java.nio.file.Path;
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.*;
Expand Down Expand Up @@ -58,8 +57,10 @@ public class EclipsePreferences implements IEclipsePreferences, IScope {
private static final String BACKUP_FILE_EXTENSION = ".bak"; //$NON-NLS-1$

private String cachedPath;
protected ImmutableMap properties = ImmutableMap.EMPTY;
protected Map<String, Object> children;
/** synchronized by childAndPropertyLock */
private ImmutableMap properties = ImmutableMap.EMPTY;
/** synchronized by childAndPropertyLock */
private Map<String, Object> children;
/**
* Protects write access to properties and children.
*/
Expand Down Expand Up @@ -125,12 +126,11 @@ public void accept(IPreferenceNodeVisitor visitor) throws BackingStoreException
}

protected IEclipsePreferences addChild(String childName, IEclipsePreferences child) {
// Thread safety: synchronize method to protect modification of children field
synchronized (childAndPropertyLock) {
if (children == null) {
children = new ConcurrentHashMap<>();
children = new HashMap<>();
}
children.put(childName, child == null ? (Object) childName : child);
children.put(childName, child == null ? childName : child);
return child;
}
}
Expand Down Expand Up @@ -469,7 +469,7 @@ protected boolean childExists(String childName) {
if (children == null) {
return false;
}
return children.get(childName) != null;
return children.containsKey(childName);
}
}

Expand All @@ -493,8 +493,8 @@ protected IEclipsePreferences getChild(String key, Object context, boolean creat
if (!create) {
return null;
}
return addChild(key, create(this, key, context));
}
return addChild(key, create(this, key, context));
}

/**
Expand Down Expand Up @@ -1144,4 +1144,29 @@ public String toString() {
void setDescriptor(ScopeDescriptor descriptor) {
this.descriptor = descriptor;
}

protected IEclipsePreferences getOrCreate(String scope) {
IEclipsePreferences child;
synchronized (childAndPropertyLock) {
if (children == null) {
child = null;
} else {
Object value = children.get(scope);
if (value == null) {
child = null;
} else if (value instanceof IEclipsePreferences eclipsePreferences) {
child = eclipsePreferences;
} else {
// lazy initialization
child = PreferencesService.getDefault().createNode(scope);
addChild(scope, child);
}
}
if (child == null) {
child = new EclipsePreferences(this, scope);
addChild(scope, child);
}
}
return child;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ public IStatus applyPreferences(IExportedPreferences preferences) throws CoreExc

// iterate over the preferences in this node and set them
// in the global space.
String[] keys = epNode.properties.keys();
String[] keys = epNode.keys();

// if this node was removed then we need to create a new one
if (removed) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,23 +48,6 @@ public void flush() throws BackingStoreException {
}
}

private synchronized IEclipsePreferences getChild(String key) {
if (children == null) {
return null;
}
Object value = children.get(key);
if (value == null) {
return null;
}
if (value instanceof IEclipsePreferences eclipsePreferences) {
return eclipsePreferences;
}
// lazy initialization
IEclipsePreferences child = PreferencesService.getDefault().createNode(key);
addChild(key, child);
return child;
}

@Override
public Preferences node(String path) {
return getNode(path, true); // create if not found
Expand All @@ -79,11 +62,7 @@ public Preferences getNode(String path, boolean create) {
String scope = path.substring(startIndex, endIndex == -1 ? path.length() : endIndex);
IEclipsePreferences child;
if (create) {
child = getChild(scope);
if (child == null) {
child = new EclipsePreferences(this, scope);
addChild(scope, child);
}
child = getOrCreate(scope);
} else {
child = getChild(scope, null, false);
if (child == null) {
Expand Down

0 comments on commit bd32d3e

Please sign in to comment.