reduce memory leak with Positions of dynamically created Text nodes
Signed-off-by: Florian Thienel <florian@thienel.org>
diff --git a/org.eclipse.vex.core.tests/src/org/eclipse/vex/core/internal/dom/GapContentTest.java b/org.eclipse.vex.core.tests/src/org/eclipse/vex/core/internal/dom/GapContentTest.java
index fec60b8..c24013c 100644
--- a/org.eclipse.vex.core.tests/src/org/eclipse/vex/core/internal/dom/GapContentTest.java
+++ b/org.eclipse.vex.core.tests/src/org/eclipse/vex/core/internal/dom/GapContentTest.java
@@ -11,6 +11,7 @@
package org.eclipse.vex.core.internal.dom;
import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertSame;
import static org.junit.Assert.fail;
import org.eclipse.core.runtime.AssertionFailedException;
@@ -40,6 +41,17 @@
}
@Test
+ public void givenAnOffset_whenInvokedMultipleTimes_shouldNotCreateMultiplePositionInstances() throws Exception {
+ final GapContent gapContent = new GapContent(4);
+ gapContent.insertElementMarker(0);
+ final Position firstPosition = gapContent.createPosition(0);
+ assertEquals(1, gapContent.getPositionCount());
+ final Position secondPosition = gapContent.createPosition(0);
+ assertEquals(1, gapContent.getPositionCount());
+ assertSame(firstPosition, secondPosition);
+ }
+
+ @Test
public void testGapContent() throws Exception {
//
// a b (gap) c d
diff --git a/org.eclipse.vex.core/src/org/eclipse/vex/core/internal/dom/Document.java b/org.eclipse.vex.core/src/org/eclipse/vex/core/internal/dom/Document.java
index 20a2bf0..c1fe65c 100644
--- a/org.eclipse.vex.core/src/org/eclipse/vex/core/internal/dom/Document.java
+++ b/org.eclipse.vex.core/src/org/eclipse/vex/core/internal/dom/Document.java
@@ -155,6 +155,10 @@
return getContent().createPosition(offset);
}
+ public void removePosition(final Position position) {
+ getContent().removePosition(position);
+ }
+
/*
* L1 Operations
*/
diff --git a/org.eclipse.vex.core/src/org/eclipse/vex/core/internal/dom/GapContent.java b/org.eclipse.vex.core/src/org/eclipse/vex/core/internal/dom/GapContent.java
index 83b72b1..f2e2b5f 100644
--- a/org.eclipse.vex.core/src/org/eclipse/vex/core/internal/dom/GapContent.java
+++ b/org.eclipse.vex.core/src/org/eclipse/vex/core/internal/dom/GapContent.java
@@ -11,8 +11,8 @@
*******************************************************************************/
package org.eclipse.vex.core.internal.dom;
-import java.util.HashSet;
-import java.util.Set;
+import java.util.SortedSet;
+import java.util.TreeSet;
import org.eclipse.core.runtime.Assert;
@@ -29,7 +29,7 @@
private char[] content;
private int gapStart;
private int gapEnd;
- private final Set<GapContentPosition> positions = new HashSet<GapContentPosition>();
+ private final SortedSet<GapContentPosition> positions = new TreeSet<GapContentPosition>();
/**
* Class constructor.
@@ -55,22 +55,36 @@
assertOffset(offset, 0, length());
- final GapContentPosition position = new GapContentPosition(offset);
- positions.add(position);
-
- return position;
+ final GapContentPosition newPosition = new GapContentPosition(offset);
+ if (positions.contains(newPosition)) {
+ final SortedSet<GapContentPosition> tailSet = positions.tailSet(newPosition);
+ final GapContentPosition storedPosition = tailSet.first();
+ storedPosition.increaseUse();
+ return storedPosition;
+ }
+ positions.add(newPosition);
+ return newPosition;
}
public void removePosition(final Position position) {
- if (positions.remove(position)) {
+ if (positions.contains(position)) {
/*
* This cast is save: if the position can be removed, this instance must have created it, hence it is a
* GapContentPosition.
*/
- ((GapContentPosition) position).invalidate();
+ final SortedSet<GapContentPosition> tailSet = positions.tailSet((GapContentPosition) position);
+ final GapContentPosition storedPosition = tailSet.first();
+ storedPosition.decreaseUse();
+ if (!storedPosition.isValid()) {
+ positions.remove(storedPosition);
+ }
}
}
+ public int getPositionCount() {
+ return positions.size();
+ }
+
/**
* Insert a string into the content.
*
@@ -102,7 +116,8 @@
if (!atEnd) {
// Update positions
- for (final GapContentPosition position : positions) {
+ final GapContentPosition offsetPosition = new GapContentPosition(offset);
+ for (final GapContentPosition position : positions.tailSet(offsetPosition)) {
if (position.getOffset() >= offset) {
position.setOffset(position.getOffset() + s.length());
}
@@ -290,14 +305,14 @@
private static final int GROWTH_RATE_FAST = 2;
private static final float GROWTH_RATE_SLOW = 1.1f;
- /**
+ /*
* Implementation of the Position interface.
*/
- private static class GapContentPosition implements Position {
+ private static class GapContentPosition implements Position, Comparable<Position> {
private int offset;
- private boolean valid = true;
+ private int useCount = 1;
public GapContentPosition(final int offset) {
this.offset = offset;
@@ -311,18 +326,52 @@
this.offset = offset;
}
+ public void increaseUse() {
+ useCount++;
+ }
+
+ public void decreaseUse() {
+ useCount--;
+ }
+
public boolean isValid() {
- return valid;
+ return useCount > 0;
};
- public void invalidate() {
- valid = false;
+ @Override
+ public int hashCode() {
+ final int prime = 31;
+ int result = 1;
+ result = prime * result + offset;
+ return result;
+ }
+
+ @Override
+ public boolean equals(final Object obj) {
+ if (this == obj) {
+ return true;
+ }
+ if (obj == null) {
+ return false;
+ }
+ if (getClass() != obj.getClass()) {
+ return false;
+ }
+ final GapContentPosition other = (GapContentPosition) obj;
+ if (offset != other.offset) {
+ return false;
+ }
+ return true;
}
@Override
public String toString() {
return Integer.toString(offset);
}
+
+ public int compareTo(final Position other) {
+ return offset - other.getOffset();
+ }
}
/**