aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorOleg Besedin2011-10-05 11:34:21 (EDT)
committerOleg Besedin2011-10-05 15:01:31 (EDT)
commitc8fc7f3886a4eac187d304f106637bf24a9331ee (patch)
treed28c5c9fe20727f94aee8c3a0ba8b7117ff2165a
parentb83abc04227c706cc4dc03b242f7f65befab9224 (diff)
downloadeclipse.platform.runtime-c8fc7f3886a4eac187d304f106637bf24a9331ee.zip
eclipse.platform.runtime-c8fc7f3886a4eac187d304f106637bf24a9331ee.tar.gz
eclipse.platform.runtime-c8fc7f3886a4eac187d304f106637bf24a9331ee.tar.bz2
Bug 358658 - Roughly 40% of the heap being eaten by EclipseContextsv20111005-1901
-rw-r--r--bundles/org.eclipse.e4.core.contexts/src/org/eclipse/e4/core/internal/contexts/Computation.java21
-rw-r--r--bundles/org.eclipse.e4.core.contexts/src/org/eclipse/e4/core/internal/contexts/EclipseContext.java155
-rw-r--r--bundles/org.eclipse.e4.core.contexts/src/org/eclipse/e4/core/internal/contexts/TrackableComputationExt.java2
-rw-r--r--bundles/org.eclipse.e4.core.contexts/src/org/eclipse/e4/core/internal/contexts/ValueComputation.java19
-rw-r--r--tests/org.eclipse.e4.core.tests/src/org/eclipse/e4/core/internal/tests/contexts/EclipseContextTest.java11
5 files changed, 82 insertions, 126 deletions
diff --git a/bundles/org.eclipse.e4.core.contexts/src/org/eclipse/e4/core/internal/contexts/Computation.java b/bundles/org.eclipse.e4.core.contexts/src/org/eclipse/e4/core/internal/contexts/Computation.java
index 8a7c365..caa28eb 100644
--- a/bundles/org.eclipse.e4.core.contexts/src/org/eclipse/e4/core/internal/contexts/Computation.java
+++ b/bundles/org.eclipse.e4.core.contexts/src/org/eclipse/e4/core/internal/contexts/Computation.java
@@ -14,24 +14,14 @@ import java.util.Set;
import org.eclipse.e4.core.internal.contexts.EclipseContext.Scheduled;
abstract public class Computation {
-
- abstract protected int calcHashCode();
- abstract public boolean equals(Object obj);
- /* final */ protected int hashCode;
- protected boolean validComputation = true;
+ abstract protected int calcHashCode();
- public void handleInvalid(ContextChangeEvent event, Set<Scheduled> scheduled) {
- invalidateComputation();
- }
+ abstract public boolean equals(Object obj);
- public boolean isValid() {
- return validComputation;
- }
+ /* final */protected int hashCode;
- public void invalidateComputation() {
- validComputation = false;
- }
+ abstract public void handleInvalid(ContextChangeEvent event, Set<Scheduled> scheduled);
@Override
public int hashCode() {
@@ -42,5 +32,4 @@ abstract public class Computation {
hashCode = calcHashCode();
}
-
-} \ No newline at end of file
+}
diff --git a/bundles/org.eclipse.e4.core.contexts/src/org/eclipse/e4/core/internal/contexts/EclipseContext.java b/bundles/org.eclipse.e4.core.contexts/src/org/eclipse/e4/core/internal/contexts/EclipseContext.java
index 09597e9..953d9ad 100644
--- a/bundles/org.eclipse.e4.core.contexts/src/org/eclipse/e4/core/internal/contexts/EclipseContext.java
+++ b/bundles/org.eclipse.e4.core.contexts/src/org/eclipse/e4/core/internal/contexts/EclipseContext.java
@@ -22,7 +22,6 @@ import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.Stack;
-import java.util.WeakHashMap;
import org.eclipse.e4.core.contexts.IContextFunction;
import org.eclipse.e4.core.contexts.IEclipseContext;
import org.eclipse.e4.core.contexts.RunAndTrack;
@@ -73,7 +72,7 @@ public class EclipseContext implements IEclipseContext {
}
}
- private Map<String, WeakHashMap<Computation, Object>> listeners = Collections.synchronizedMap(new HashMap<String, WeakHashMap<Computation, Object>>(10, 0.8f));
+ private Map<String, HashSet<Computation>> listeners = Collections.synchronizedMap(new HashMap<String, HashSet<Computation>>(10, 0.8f));
private Map<String, ValueComputation> localValueComputations = Collections.synchronizedMap(new HashMap<String, ValueComputation>());
private Set<Computation> activeRATs = new HashSet<Computation>();
@@ -108,11 +107,14 @@ public class EclipseContext implements IEclipseContext {
debugAddOn.notify(this, IEclipseContextDebugger.EventType.CONSTRUCTED, null);
}
+ final static private Set<EclipseContext> noChildren = new HashSet<EclipseContext>(0);
+
public Set<EclipseContext> getChildren() {
- if (children.size() == 0)
- return null;
- Set<EclipseContext> result = new HashSet<EclipseContext>(children.size());
+ Set<EclipseContext> result;
synchronized (children) {
+ if (children.size() == 0)
+ return noChildren;
+ result = new HashSet<EclipseContext>(children.size());
for (Iterator<WeakReference<EclipseContext>> i = children.iterator(); i.hasNext();) {
EclipseContext referredContext = i.next().get();
if (referredContext == null) {
@@ -126,6 +128,7 @@ public class EclipseContext implements IEclipseContext {
}
public boolean containsKey(String name) {
+ trackAccess(name);
return containsKey(name, false);
}
@@ -151,30 +154,16 @@ public class EclipseContext implements IEclipseContext {
*/
public void dispose() {
// dispose of child contexts first
- EclipseContext[] currentChildren = null;
- synchronized (children) {
- if (children.size() > 0) {
- Set<EclipseContext> localCopy = new HashSet<EclipseContext>(children.size());
- for (WeakReference<EclipseContext> childContextRef : children) {
- EclipseContext childContext = childContextRef.get();
- if (childContext != null)
- localCopy.add(childContext);
- }
- currentChildren = new EclipseContext[localCopy.size()];
- localCopy.toArray(currentChildren);
- children.clear(); // just in case
- }
- }
- if (currentChildren != null) {
- for (EclipseContext childContext : currentChildren) {
- childContext.dispose();
- }
+ for (EclipseContext childContext : getChildren()) {
+ childContext.dispose();
}
ContextChangeEvent event = new ContextChangeEvent(this, ContextChangeEvent.DISPOSE, null, null, null);
Set<Scheduled> scheduled = new LinkedHashSet<Scheduled>();
Set<Computation> allComputations = getListeners();
listeners.clear();
+ allComputations.addAll(activeRATs);
+ activeRATs.clear();
for (Computation computation : allComputations) {
computation.handleInvalid(event, scheduled);
}
@@ -207,18 +196,19 @@ public class EclipseContext implements IEclipseContext {
}
public Object get(String name) {
+ trackAccess(name);
return internalGet(this, name, false);
}
public Object getLocal(String name) {
+ trackAccess(name);
return internalGet(this, name, true);
}
public Object internalGet(EclipseContext originatingContext, String name, boolean local) {
- trackAccess(name);
if (this == originatingContext) {
ValueComputation valueComputation = localValueComputations.get(name);
- if (valueComputation != null && valueComputation.isValid())
+ if (valueComputation != null)
return valueComputation.get();
}
@@ -235,26 +225,15 @@ public class EclipseContext implements IEclipseContext {
if (result != null) {
if (result instanceof IContextFunction) {
ValueComputation valueComputation = new ValueComputation(name, originatingContext, ((IContextFunction) result));
-
// do calculations before adding listeners
result = valueComputation.get();
-
originatingContext.localValueComputations.put(name, valueComputation);
-
- // need to manually add dependency as the computation haven't being created yet at the time
- // we walked context hierarchy to find its definition
- for (EclipseContext step = originatingContext; step != null; step = step.getParent()) {
- step.addDependency(name, valueComputation);
- if (step == this)
- break;
- step.addDependency(PARENT, valueComputation);
- }
}
return result;
}
// 3. delegate to parent
if (!local) {
- IEclipseContext parent = (IEclipseContext) getLocal(PARENT);
+ IEclipseContext parent = (IEclipseContext) localValues.get(PARENT);
if (parent != null) {
return ((EclipseContext) parent).internalGet(originatingContext, name, local);
}
@@ -269,35 +248,30 @@ public class EclipseContext implements IEclipseContext {
public void invalidate(String name, int eventType, Object oldValue, Set<Scheduled> scheduled) {
ContextChangeEvent event = new ContextChangeEvent(this, eventType, null, name, oldValue);
ValueComputation computation = localValueComputations.get(name);
- if (computation != null && computation.isValid()) {
+ if (computation != null) {
+ if (computation.shouldRemove(event)) {
+ localValueComputations.remove(name);
+ Collection<HashSet<Computation>> allListeners = listeners.values();
+ for (HashSet<Computation> group : allListeners) {
+ group.remove(computation);
+ }
+ }
computation.handleInvalid(event, scheduled);
}
- WeakHashMap<Computation, Object> namedComputations = listeners.get(name);
+ HashSet<Computation> namedComputations = listeners.get(name);
if (namedComputations != null) {
- int invalidListenersCount = 0;
- for (Computation listener : namedComputations.keySet()) {
- if (listener.isValid())
- listener.handleInvalid(event, scheduled);
- else
- invalidListenersCount++;
- }
- if (invalidListenersCount == namedComputations.size()) {
- // all the listeners are invalid for this name
- listeners.remove(name);
- } else if (invalidListenersCount > 10 && (invalidListenersCount << 1) > namedComputations.size()) {
- // more than half of listeners are invalid, clean the listener list
- WeakHashMap<Computation, Object> tmp = new WeakHashMap<Computation, Object>(namedComputations.size() - invalidListenersCount, 0.75f);
- for (Computation listener : namedComputations.keySet()) {
- if (listener.isValid())
- tmp.put(listener, null);
- }
- listeners.put(name, tmp);
+ for (Computation listener : namedComputations) {
+ listener.handleInvalid(event, scheduled);
}
}
+
+ // invalidate this name in child contexts
+ for (EclipseContext childContext : getChildren()) {
+ childContext.invalidate(name, eventType, oldValue, scheduled);
+ }
}
private boolean isSetLocally(String name) {
- trackAccess(name);
return localValues.containsKey(name);
}
@@ -320,6 +294,11 @@ public class EclipseContext implements IEclipseContext {
public void removeRAT(Computation computation) {
activeRATs.remove(computation);
+ // also remove from listeners
+ Collection<HashSet<Computation>> allListeners = listeners.values();
+ for (HashSet<Computation> group : allListeners) {
+ group.remove(computation);
+ }
}
protected void processScheduled(Set<Scheduled> scheduledList) {
@@ -330,9 +309,6 @@ public class EclipseContext implements IEclipseContext {
}
public void set(String name, Object value) {
- if (DebugHelper.DEBUG_NAMES)
- System.out.println("[context] set(" + name + ',' + value + ")" + " on " + toString());//$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$
-
if (PARENT.equals(name)) {
setParent((IEclipseContext) value);
return;
@@ -373,7 +349,6 @@ public class EclipseContext implements IEclipseContext {
}
public EclipseContext getParent() {
- trackAccess(PARENT);
return (EclipseContext) localValues.get(PARENT);
}
@@ -411,15 +386,12 @@ public class EclipseContext implements IEclipseContext {
}
public void addDependency(String name, Computation computation) {
- WeakHashMap<Computation, Object> nameListeners = listeners.get(name);
+ HashSet<Computation> nameListeners = listeners.get(name);
if (nameListeners == null) {
- nameListeners = new WeakHashMap<Computation, Object>(30, 0.75f);
+ nameListeners = new HashSet<Computation>(30, 0.75f);
listeners.put(name, nameListeners);
}
- // XXX a new computation (valid) might be equals to an old computation (invalid)
- if (nameListeners.containsKey(computation))
- nameListeners.remove(computation);
- nameListeners.put(computation, null);
+ nameListeners.add(computation);
}
public void declareModifiable(String name) {
@@ -456,14 +428,11 @@ public class EclipseContext implements IEclipseContext {
}
public Set<Computation> getListeners() {
- Collection<WeakHashMap<Computation, Object>> collection = listeners.values();
+ Collection<HashSet<Computation>> collection = listeners.values();
Set<Computation> comps = new HashSet<Computation>();
- for (WeakHashMap<Computation, Object> map : collection) {
- for (Computation comp : map.keySet()) {
- if (comp.isValid())
- comps.add(comp);
- }
+ for (HashSet<Computation> tmp : collection) {
+ comps.addAll(tmp);
}
return comps;
}
@@ -473,9 +442,8 @@ public class EclipseContext implements IEclipseContext {
// Add "boolean inReparent" on the root context and process right away?
processWaiting();
// 1) everybody who depends on me: I need to collect combined list of names injected
- Set<String> tmp = listeners.keySet(); // clone internal name list
- Set<String> usedNames = new HashSet<String>(tmp.size());
- usedNames.addAll(tmp);
+ Set<String> usedNames = new HashSet<String>();
+ collectDependentNames(usedNames);
// 2) for each used name:
for (Iterator<String> i = usedNames.iterator(); i.hasNext();) {
@@ -487,8 +455,25 @@ public class EclipseContext implements IEclipseContext {
if (oldValue != newValue)
invalidate(name, ContextChangeEvent.ADDED, oldValue, scheduled);
}
+
+ ContextChangeEvent event = new ContextChangeEvent(this, ContextChangeEvent.ADDED, null, null, null);
+ for (Computation computation : localValueComputations.values()) {
+ Collection<HashSet<Computation>> allListeners = listeners.values();
+ for (HashSet<Computation> group : allListeners) {
+ group.remove(computation);
+ }
+ computation.handleInvalid(event, scheduled);
+ }
localValueComputations.clear();
- // XXX localValueComputations -> all invalidate
+ }
+
+ private void collectDependentNames(Set<String> usedNames) {
+ Set<String> tmp = listeners.keySet(); // clone internal name list
+ usedNames.addAll(tmp);
+
+ for (EclipseContext childContext : getChildren()) {
+ childContext.collectDependentNames(usedNames);
+ }
}
public void processWaiting() {
@@ -599,6 +584,7 @@ public class EclipseContext implements IEclipseContext {
}
public IEclipseContext getActiveChild() {
+ trackAccess(ACTIVE_CHILD);
return (EclipseContext) internalGet(this, ACTIVE_CHILD, true);
}
@@ -664,10 +650,8 @@ public class EclipseContext implements IEclipseContext {
Map<String, Object> result = new HashMap<String, Object>(localValueComputations.size());
for (String string : localValueComputations.keySet()) {
ValueComputation vc = localValueComputations.get(string);
- if (vc == null)
- continue;
- if (vc.isValid())
- result.put(string, localValueComputations.get(string).get());
+ if (vc != null)
+ result.put(string, vc.get());
}
return result;
}
@@ -682,14 +666,11 @@ public class EclipseContext implements IEclipseContext {
// This method is for debug only, do not use externally
public Set<Computation> getListeners(String name) {
- WeakHashMap<Computation, Object> tmp = listeners.get(name);
+ HashSet<Computation> tmp = listeners.get(name);
if (tmp == null)
return null;
Set<Computation> result = new HashSet<Computation>(tmp.size());
- for (Computation listener : tmp.keySet()) {
- if (listener.isValid())
- result.add(listener);
- }
+ result.addAll(tmp);
return result;
}
diff --git a/bundles/org.eclipse.e4.core.contexts/src/org/eclipse/e4/core/internal/contexts/TrackableComputationExt.java b/bundles/org.eclipse.e4.core.contexts/src/org/eclipse/e4/core/internal/contexts/TrackableComputationExt.java
index db6b3a4..61d93d8 100644
--- a/bundles/org.eclipse.e4.core.contexts/src/org/eclipse/e4/core/internal/contexts/TrackableComputationExt.java
+++ b/bundles/org.eclipse.e4.core.contexts/src/org/eclipse/e4/core/internal/contexts/TrackableComputationExt.java
@@ -117,13 +117,11 @@ public class TrackableComputationExt extends Computation {
if (eventType == ContextChangeEvent.DISPOSE) {
if (originatingContext.equals(eventsContext)) {
((EclipseContext) originatingContext).removeRAT(this);
- invalidateComputation();
return false;
}
}
if (!result) {
((EclipseContext) originatingContext).removeRAT(this);
- invalidateComputation();
}
return result;
}
diff --git a/bundles/org.eclipse.e4.core.contexts/src/org/eclipse/e4/core/internal/contexts/ValueComputation.java b/bundles/org.eclipse.e4.core.contexts/src/org/eclipse/e4/core/internal/contexts/ValueComputation.java
index a935d59..ec994d2 100644
--- a/bundles/org.eclipse.e4.core.contexts/src/org/eclipse/e4/core/internal/contexts/ValueComputation.java
+++ b/bundles/org.eclipse.e4.core.contexts/src/org/eclipse/e4/core/internal/contexts/ValueComputation.java
@@ -34,24 +34,20 @@ public class ValueComputation extends Computation {
}
public void handleInvalid(ContextChangeEvent event, Set<Scheduled> scheduled) {
- int eventType = event.getEventType();
-
- boolean containerDisposed = (eventType == ContextChangeEvent.DISPOSE && event.getContext() == originatingContext);
- boolean definitionChanged = (eventType != ContextChangeEvent.RECALC && name.equals(event.getName()));
-
- if (containerDisposed || definitionChanged)
- invalidateComputation();
-
if (cachedValue == NotAValue) // already invalidated
return;
-
cachedValue = NotAValue;
originatingContext.invalidate(name, ContextChangeEvent.RECALC, event.getOldValue(), scheduled);
}
+ public boolean shouldRemove(ContextChangeEvent event) {
+ int eventType = event.getEventType();
+ boolean containerDisposed = (eventType == ContextChangeEvent.DISPOSE && event.getContext() == originatingContext);
+ boolean definitionChanged = (eventType != ContextChangeEvent.RECALC && name.equals(event.getName()));
+ return (containerDisposed || definitionChanged);
+ }
+
public Object get() {
- if (!isValid())
- throw new IllegalArgumentException("Reusing invalidated computation"); //$NON-NLS-1$
if (cachedValue != NotAValue)
return cachedValue;
if (this.computing)
@@ -61,7 +57,6 @@ public class ValueComputation extends Computation {
computing = true;
try {
cachedValue = function.compute(originatingContext);
- validComputation = true;
} finally {
computing = false;
originatingContext.popComputation(this);
diff --git a/tests/org.eclipse.e4.core.tests/src/org/eclipse/e4/core/internal/tests/contexts/EclipseContextTest.java b/tests/org.eclipse.e4.core.tests/src/org/eclipse/e4/core/internal/tests/contexts/EclipseContextTest.java
index abfc959..44468f3 100644
--- a/tests/org.eclipse.e4.core.tests/src/org/eclipse/e4/core/internal/tests/contexts/EclipseContextTest.java
+++ b/tests/org.eclipse.e4.core.tests/src/org/eclipse/e4/core/internal/tests/contexts/EclipseContextTest.java
@@ -92,7 +92,7 @@ public class EclipseContextTest extends TestCase {
assertEquals("bar", context.get("foo"));
context.dispose();
assertNull(context.get("foo"));
- assertNull(((EclipseContext)parentContext).getChildren());
+ assertTrue(((EclipseContext)parentContext).getChildren().isEmpty());
}
/**
@@ -208,13 +208,6 @@ public class EclipseContextTest extends TestCase {
assertEquals(2, runCounter);
parent.set("parentValue", "z");
assertEquals(3, runCounter);
- // at this point we should no longer be listening
- child.set("childValue", "y");
- assertEquals(3, runCounter);
- parent.set("parentValue", "y");
- assertEquals(3, runCounter);
- assertEquals(0, listenersCount(child));
- assertEquals(0, listenersCount(parent));
}
public void testModify() {
@@ -306,7 +299,7 @@ public class EclipseContextTest extends TestCase {
parent.set("sum", new AddContextFunction());
child.get("sum");
- assertEquals(1, listenersCount(parent));
+ assertEquals(1, listenersCount(child));
child.dispose();
assertEquals(0, listenersCount(parent));
}