The StringContent class has been plagued by the same problems as the
GapContent class, as was pointed out by the Intel testsuite:
- Position instances couldn't be garbage collected, resulting in memory
leakage.
- Undo/Redo didn't update the Positions
Both issues have been fixed. This was mostly adapting/copy+pasting code
from GapContent to fit with StringContent. Now we pass Intel's testsuite
for this class, except for one case, where we don't really want to be
bug compatible. Not that I also didn't bother optimizing the
position/mark handling to be 100% optimal wrt memory usage (we could
share Marks between positions, as is done in GapContent for example. and
we could sort them for faster search etc). I think the StringContent
class is hardly used by any application anyway.
2006-08-27 Roman Kennke <[EMAIL PROTECTED]>
* javax/swing/text/StringContent.java
(InsertUndo.positions): New field.
(InsertUndo.redo): Update the undo positions.
(InsertUndo.undo): Fetch the undo positions.
(Mark): New class. Layer of indirection to allow Positions
to be GC'ed while we still hold references to the Mark.
(RemoveUndo.len): New field.
(RemoveUndo.positions): New field.
(RemoveUndo.RemoveUndo): Fetch undo positions.
(RemoveUndo.redo): Re-fetch positions and string.
(RemoveUndo.undo): Update undo positions.
(StickyPosition.mark): New field.
(StickyPosition.offset): Removed field.
(StickyPosition.StickyPosition): Create new Mark. Register
Position in queueOfDeath. Update reference count on mark.
(StickyPosition.getOffset): Return offset stored in mark.
(StickyPosition.setOffset): Removed unneeded method.
(UndoPosRef): New class. Handles undo/redo on positions/marks.
(EMPTY): New field.
(marks): New field. Stores the marks.
(positions): Removed field.
(queueOfDeath): New field. Used for GCing the positions.
(StringContent): Initialize queueOfDeath.
(createPosition): Lazily create marks vector.
(garbageCollect): New helper method. Collects positions
to be GCed and updates their marks.
(getChars): Fixed bounds check.
(getPositionsInRange): When v == null, create new Vector,
otherwise use v. Store UndoPosRefs in vector.
(getString): Added comment about bug in RI.
(insertString): Use new helper method for replacing the array.
Correctly update positions.
(length): Removed this qualifier.
(remove): Use new helper method for replacing the array.
Correctly update positions.
(replace): New helper method for growing or patching the array.
(updateUndoPositions): Implemented. Updates the positions
for undo/redo operations.
/Roman
Index: javax/swing/text/StringContent.java
===================================================================
RCS file: /cvsroot/classpath/classpath/javax/swing/text/StringContent.java,v
retrieving revision 1.9
diff -u -1 -2 -r1.9 StringContent.java
--- javax/swing/text/StringContent.java 26 Jul 2006 07:36:41 -0000 1.9
+++ javax/swing/text/StringContent.java 28 Aug 2006 09:52:53 -0000
@@ -30,381 +30,541 @@
terms of your choice, provided that you also meet, for each linked
independent module, the terms and conditions of the license of that
module. An independent module is a module which is not derived from
or based on this library. If you modify this library, you may extend
this exception to your version of the library, but you are not
obligated to do so. If you do not wish to do so, delete this
exception statement from your version. */
package javax.swing.text;
import java.io.Serializable;
+import java.lang.ref.Reference;
+import java.lang.ref.ReferenceQueue;
+import java.lang.ref.WeakReference;
import java.util.Iterator;
import java.util.Vector;
import javax.swing.undo.AbstractUndoableEdit;
import javax.swing.undo.CannotRedoException;
import javax.swing.undo.CannotUndoException;
import javax.swing.undo.UndoableEdit;
/**
* An implementation of the <code>AbstractDocument.Content</code>
* interface useful for small documents or debugging. The character
* content is a simple character array. It's not really efficient.
*
* <p>Do not use this class for large size.</p>
*/
public final class StringContent
implements AbstractDocument.Content, Serializable
{
+ /**
+ * Stores a reference to a mark that can be resetted to the original value
+ * after a mark has been moved. This is used for undoing actions.
+ */
+ private class UndoPosRef
+ {
+ /**
+ * The mark that might need to be reset.
+ */
+ private Mark mark;
+
+ /**
+ * The original offset to reset the mark to.
+ */
+ private int undoOffset;
+
+ /**
+ * Creates a new UndoPosRef.
+ *
+ * @param m the mark
+ */
+ UndoPosRef(Mark m)
+ {
+ mark = m;
+ undoOffset = mark.mark;
+ }
+
+ /**
+ * Resets the position of the mark to the value that it had when
+ * creating this UndoPosRef.
+ */
+ void reset()
+ {
+ mark.mark = undoOffset;
+ }
+ }
+
+ /**
+ * Holds a mark into the buffer that is used by StickyPosition to find
+ * the actual offset of the position. This is pulled out of the
+ * GapContentPosition object so that the mark and position can be handled
+ * independently, and most important, so that the StickyPosition can
+ * be garbage collected while we still hold a reference to the Mark object.
+ */
+ private class Mark
+ {
+ /**
+ * The actual mark into the buffer.
+ */
+ int mark;
+
+
+ /**
+ * The number of GapContentPosition object that reference this mark. If
+ * it reaches zero, it get's deleted by
+ * [EMAIL PROTECTED] StringContent#garbageCollect()}.
+ */
+ int refCount;
+
+ /**
+ * Creates a new Mark object for the specified offset.
+ *
+ * @param offset the offset
+ */
+ Mark(int offset)
+ {
+ mark = offset;
+ }
+ }
+
/** The serialization UID (compatible with JDK1.5). */
private static final long serialVersionUID = 4755994433709540381L;
// This is package-private to avoid an accessor method.
char[] content;
private int count;
- private Vector positions = new Vector();
+ /**
+ * Holds the marks for the positions.
+ *
+ * This is package private to avoid accessor methods.
+ */
+ Vector marks;
private class InsertUndo extends AbstractUndoableEdit
{
private int start;
private int length;
private String redoContent;
+ private Vector positions;
+
public InsertUndo(int start, int length)
{
super();
this.start = start;
this.length = length;
}
public void undo()
{
super.undo();
try
{
- StringContent.this.checkLocation(this.start, this.length);
- this.redoContent = new String(StringContent.this.content, this.start,
- this.length);
- StringContent.this.remove(this.start, this.length);
+ if (marks != null)
+ positions = getPositionsInRange(null, start, length);
+ redoContent = getString(start, length);
+ remove(start, length);
}
catch (BadLocationException b)
{
throw new CannotUndoException();
}
}
public void redo()
{
super.redo();
try
{
- StringContent.this.insertString(this.start, this.redoContent);
+ insertString(start, redoContent);
+ redoContent = null;
+ if (positions != null)
+ {
+ updateUndoPositions(positions);
+ positions = null;
+ }
}
catch (BadLocationException b)
{
throw new CannotRedoException();
}
}
}
private class RemoveUndo extends AbstractUndoableEdit
{
private int start;
-
+ private int len;
private String undoString;
+ Vector positions;
+
public RemoveUndo(int start, String str)
{
super();
this.start = start;
+ len = str.length();
this.undoString = str;
+ if (marks != null)
+ positions = getPositionsInRange(null, start, str.length());
}
public void undo()
{
super.undo();
try
{
StringContent.this.insertString(this.start, this.undoString);
+ if (positions != null)
+ {
+ updateUndoPositions(positions);
+ positions = null;
+ }
+ undoString = null;
}
catch (BadLocationException bad)
{
throw new CannotUndoException();
}
}
public void redo()
{
super.redo();
try
{
- int end = this.undoString.length();
- StringContent.this.remove(this.start, end);
+ undoString = getString(start, len);
+ if (marks != null)
+ positions = getPositionsInRange(null, start, len);
+ remove(this.start, len);
}
catch (BadLocationException bad)
{
throw new CannotRedoException();
}
}
}
private class StickyPosition implements Position
{
- private int offset = -1;
+ Mark mark;
public StickyPosition(int offset)
{
- this.offset = offset;
- }
+ // Try to make space.
+ garbageCollect();
- // This is package-private to avoid an accessor method.
- void setOffset(int offset)
- {
- this.offset = this.offset >= 0 ? offset : -1;
+ mark = new Mark(offset);
+ mark.refCount++;
+ marks.add(mark);
+
+ new WeakReference(this, queueOfDeath);
}
/**
* Should be >=0.
*/
public int getOffset()
{
- return offset < 0 ? 0 : offset;
+ return mark.mark;
}
}
/**
+ * Used in [EMAIL PROTECTED] #remove(int,int)}.
+ */
+ private static final char[] EMPTY = new char[0];
+
+ /**
+ * Queues all references to GapContentPositions that are about to be
+ * GC'ed. This is used to remove the corresponding marks from the
+ * positionMarks array if the number of references to that mark reaches zero.
+ *
+ * This is package private to avoid accessor synthetic methods.
+ */
+ ReferenceQueue queueOfDeath;
+
+ /**
* Creates a new instance containing the string "\n". This is equivalent
* to calling [EMAIL PROTECTED] #StringContent(int)} with an <code>initialLength</code>
* of 10.
*/
public StringContent()
{
this(10);
}
/**
* Creates a new instance containing the string "\n".
*
* @param initialLength the initial length of the underlying character
* array used to store the content.
*/
public StringContent(int initialLength)
{
super();
+ queueOfDeath = new ReferenceQueue();
if (initialLength < 1)
initialLength = 1;
this.content = new char[initialLength];
this.content[0] = '\n';
this.count = 1;
}
protected Vector getPositionsInRange(Vector v,
int offset,
int length)
{
- Vector refPos = new Vector();
- Iterator iter = this.positions.iterator();
+ Vector refPos = v == null ? new Vector() : v;
+ Iterator iter = marks.iterator();
while(iter.hasNext())
{
- Position p = (Position) iter.next();
- if ((offset <= p.getOffset())
- && (p.getOffset() <= (offset + length)))
- refPos.add(p);
+ Mark m = (Mark) iter.next();
+ if (offset <= m.mark && m.mark <= offset + length)
+ refPos.add(new UndoPosRef(m));
}
return refPos;
}
/**
* Creates a position reference for the character at the given offset. The
* position offset will be automatically updated when new characters are
* inserted into or removed from the content.
*
* @param offset the character offset.
*
* @throws BadLocationException if offset is outside the bounds of the
* content.
*/
public Position createPosition(int offset) throws BadLocationException
{
- if (offset < this.count || offset > this.count)
- checkLocation(offset, 0);
+ // Lazily create marks vector.
+ if (marks == null)
+ marks = new Vector();
StickyPosition sp = new StickyPosition(offset);
- this.positions.add(sp);
return sp;
}
/**
* Returns the length of the string content, including the '\n' character at
* the end.
*
* @return The length of the string content.
*/
public int length()
{
- return this.count;
+ return count;
}
/**
* Inserts <code>str</code> at the given position and returns an
* [EMAIL PROTECTED] UndoableEdit} that enables undo/redo support.
*
* @param where the insertion point (must be less than
* <code>length()</code>).
* @param str the string to insert (<code>null</code> not permitted).
*
* @return An object that can undo the insertion.
*/
public UndoableEdit insertString(int where, String str)
throws BadLocationException
{
checkLocation(where, 0);
if (where == this.count)
throw new BadLocationException("Invalid location", 1);
if (str == null)
throw new NullPointerException();
char[] insert = str.toCharArray();
- char[] temp = new char[this.content.length + insert.length];
- this.count += insert.length;
- // Copy array and insert the string.
- if (where > 0)
- System.arraycopy(this.content, 0, temp, 0, where);
- System.arraycopy(insert, 0, temp, where, insert.length);
- System.arraycopy(this.content, where, temp, (where + insert.length),
- (temp.length - where - insert.length));
- if (this.content.length < temp.length)
- this.content = new char[temp.length];
- // Copy the result in the original char array.
- System.arraycopy(temp, 0, this.content, 0, temp.length);
+ replace(where, 0, insert);
+
// Move all the positions.
- Vector refPos = getPositionsInRange(this.positions, where,
- temp.length - where);
- Iterator iter = refPos.iterator();
- while (iter.hasNext())
+ if (marks != null)
{
- StickyPosition p = (StickyPosition)iter.next();
- p.setOffset(p.getOffset() + str.length());
+ Iterator iter = marks.iterator();
+ int start = where;
+ if (start == 0)
+ start = 1;
+ while (iter.hasNext())
+ {
+ Mark m = (Mark) iter.next();
+ if (m.mark >= start)
+ m.mark += str.length();
+ }
}
+
InsertUndo iundo = new InsertUndo(where, insert.length);
return iundo;
}
/**
* Removes the specified range of characters and returns an
* [EMAIL PROTECTED] UndoableEdit} that enables undo/redo support.
*
* @param where the starting index.
* @param nitems the number of characters.
*
* @return An object that can undo the removal.
*
* @throws BadLocationException if the character range extends outside the
* bounds of the content OR includes the last character.
*/
public UndoableEdit remove(int where, int nitems) throws BadLocationException
{
checkLocation(where, nitems + 1);
- char[] temp = new char[(this.content.length - nitems)];
- this.count = this.count - nitems;
RemoveUndo rundo = new RemoveUndo(where, new String(this.content, where,
nitems));
- // Copy array.
- System.arraycopy(this.content, 0, temp, 0, where);
- System.arraycopy(this.content, where + nitems, temp, where,
- this.content.length - where - nitems);
- this.content = new char[temp.length];
- // Then copy the result in the original char array.
- System.arraycopy(temp, 0, this.content, 0, this.content.length);
+
+ replace(where, nitems, EMPTY);
// Move all the positions.
- Vector refPos = getPositionsInRange(this.positions, where,
- this.content.length + nitems - where);
- Iterator iter = refPos.iterator();
- while (iter.hasNext())
+ if (marks != null)
{
- StickyPosition p = (StickyPosition)iter.next();
- int result = p.getOffset() - nitems;
- p.setOffset(result);
- if (result < 0)
- this.positions.remove(p);
+ Iterator iter = marks.iterator();
+ while (iter.hasNext())
+ {
+ Mark m = (Mark) iter.next();
+ if (m.mark >= where + nitems)
+ m.mark -= nitems;
+ else if (m.mark >= where)
+ m.mark = where;
+ }
}
return rundo;
}
-
+
+ private void replace(int offs, int numRemove, char[] insert)
+ {
+ int insertLength = insert.length;
+ int delta = insertLength - numRemove;
+ int src = offs + numRemove;
+ int numMove = count - src;
+ int dest = src + delta;
+ if (count + delta >= content.length)
+ {
+ // Grow data array.
+ int newLength = Math.max(2 * content.length, count + delta);
+ char[] newContent = new char[newLength];
+ System.arraycopy(content, 0, newContent, 0, offs);
+ System.arraycopy(insert, 0, newContent, offs, insertLength);
+ System.arraycopy(content, src, newContent, dest, numMove);
+ content = newContent;
+ }
+ else
+ {
+ System.arraycopy(content, src, content, dest, numMove);
+ System.arraycopy(insert, 0, content, offs, insertLength);
+ }
+ count += delta;
+ }
+
/**
* Returns a new <code>String</code> containing the characters in the
* specified range.
*
* @param where the start index.
* @param len the number of characters.
*
* @return A string.
*
* @throws BadLocationException if the requested range of characters extends
* outside the bounds of the content.
*/
public String getString(int where, int len) throws BadLocationException
{
+ // The RI throws a StringIndexOutOfBoundsException here, which
+ // smells like a bug. We throw a BadLocationException instead.
checkLocation(where, len);
return new String(this.content, where, len);
}
/**
* Updates <code>txt</code> to contain a direct reference to the underlying
* character array.
*
* @param where the index of the first character.
* @param len the number of characters.
* @param txt a carrier for the return result (<code>null</code> not
* permitted).
*
* @throws BadLocationException if the requested character range is not
* within the bounds of the content.
* @throws NullPointerException if <code>txt</code> is <code>null</code>.
*/
public void getChars(int where, int len, Segment txt)
throws BadLocationException
{
- checkLocation(where, len);
- txt.array = this.content;
+ if (where + len > count)
+ throw new BadLocationException("Invalid location", where + len);
+ txt.array = content;
txt.offset = where;
txt.count = len;
}
/**
- * @specnote This method is not very well specified and the positions vector
- * is implementation specific. The undo positions are managed
- * differently in this implementation, this method is only here
- * for binary compatibility.
+ * Resets the positions in the specified vector to their original offset
+ * after a undo operation is performed. For example, after removing some
+ * content, the positions in the removed range will all be set to one
+ * offset. This method restores the positions to their original offsets
+ * after an undo.
*/
protected void updateUndoPositions(Vector positions)
{
- // We do nothing here.
+ for (Iterator i = positions.iterator(); i.hasNext();)
+ {
+ UndoPosRef pos = (UndoPosRef) i.next();
+ pos.reset();
+ }
}
/**
* A utility method that checks the validity of the specified character
* range.
*
* @param where the first character in the range.
* @param len the number of characters in the range.
*
* @throws BadLocationException if the specified range is not within the
* bounds of the content.
*/
void checkLocation(int where, int len) throws BadLocationException
{
if (where < 0)
throw new BadLocationException("Invalid location", 1);
else if (where > this.count)
throw new BadLocationException("Invalid location", this.count);
else if ((where + len) > this.count)
throw new BadLocationException("Invalid range", this.count);
}
-
+
+ /**
+ * Polls the queue of death for GapContentPositions, updates the
+ * corresponding reference count and removes the corresponding mark
+ * if the refcount reaches zero.
+ *
+ * This is package private to avoid accessor synthetic methods.
+ */
+ void garbageCollect()
+ {
+ Reference ref = queueOfDeath.poll();
+ while (ref != null)
+ {
+ if (ref != null)
+ {
+ StickyPosition pos = (StickyPosition) ref.get();
+ Mark m = pos.mark;
+ m.refCount--;
+ if (m.refCount == 0)
+ marks.remove(m);
+ }
+ ref = queueOfDeath.poll();
+ }
+ }
}
