Bug 545252 - Improve IDocumentAdapterExtension in DefaultDocumentAdapter
with copy-on-write approach
For the old implementation when change forwarding is disabled the whole
document content is copied and the line tracking information rebuild
even if the document is not changed but only read.
The new approach only copies the document if a change happens.
Change-Id: I396a142d70268e33017117e9e99535ae37463b72
Signed-off-by: Paul Pazderski <paul-eclipse@ppazderski.de>
diff --git a/org.eclipse.jface.text/src/org/eclipse/jface/text/DefaultDocumentAdapter.java b/org.eclipse.jface.text/src/org/eclipse/jface/text/DefaultDocumentAdapter.java
index cf417ce..7695905 100644
--- a/org.eclipse.jface.text/src/org/eclipse/jface/text/DefaultDocumentAdapter.java
+++ b/org.eclipse.jface.text/src/org/eclipse/jface/text/DefaultDocumentAdapter.java
@@ -1,5 +1,5 @@
/*******************************************************************************
- * Copyright (c) 2000, 2007 IBM Corporation and others.
+ * Copyright (c) 2000, 2019 IBM Corporation and others.
*
* This program and the accompanying materials
* are made available under the terms of the Eclipse Public License 2.0
@@ -10,6 +10,7 @@
*
* Contributors:
* IBM Corporation - initial API and implementation
+ * Paul Pazderski - Bug 545252 - improve stop/resumeForwarding with copy-on-write approach
*******************************************************************************/
package org.eclipse.jface.text;
@@ -36,12 +37,17 @@
/** The adapted document. */
private IDocument fDocument;
- /** The document clone for the non-forwarding case. */
- private IDocument fDocumentClone;
- /** The original content */
- private String fOriginalContent;
- /** The original line delimiters */
- private String[] fOriginalLineDelimiters;
+ /**
+ * The document to be used to read stuff. Most of the time this is equal to {@link #fDocument}.
+ * Only if non-forwarding is enabled and {@link #fDocument} was changed while non-forwarding
+ * {@link #fActiveDocument} points to a read-only copy of {@link #fDocument} with the content it
+ * had when change forwarding was disabled. I.e. this reference must always expected to be
+ * read-only.
+ *
+ * @see IDocumentAdapterExtension
+ * @see #fIsForwarding
+ */
+ private IDocument fActiveDocument;
/** The registered text change listeners */
private List<TextChangeListener> fTextChangeListeners= new ArrayList<>(1);
/**
@@ -92,19 +98,9 @@
fDocument.removePrenotifiedDocumentListener(this);
fDocument= document;
+ fActiveDocument= fDocument;
fLineDelimiter= null;
- if (!fIsForwarding) {
- fDocumentClone= null;
- if (fDocument != null) {
- fOriginalContent= fDocument.get();
- fOriginalLineDelimiters= fDocument.getLegalLineDelimiters();
- } else {
- fOriginalContent= null;
- fOriginalLineDelimiters= null;
- }
- }
-
if (fDocument != null)
fDocument.addPrenotifiedDocumentListener(this);
}
@@ -151,16 +147,7 @@
}
private IDocument getDocumentForRead() {
- if (!fIsForwarding) {
- if (fDocumentClone == null) {
- String content= fOriginalContent == null ? "" : fOriginalContent; //$NON-NLS-1$
- String[] delims= fOriginalLineDelimiters == null ? DefaultLineTracker.DELIMITERS : fOriginalLineDelimiters;
- fDocumentClone= new DocumentClone(content, delims);
- }
- return fDocumentClone;
- }
-
- return fDocument;
+ return fActiveDocument;
}
@Override
@@ -274,6 +261,9 @@
@Override
public void documentAboutToBeChanged(DocumentEvent event) {
+ if (!fIsForwarding && fDocument == fActiveDocument) {
+ fActiveDocument= new DocumentClone(fActiveDocument.get(), fActiveDocument.getLegalLineDelimiters());
+ }
fRememberedLengthOfDocument= fDocument.getLength();
try {
@@ -377,17 +367,12 @@
@Override
public void resumeForwardingDocumentChanges() {
fIsForwarding= true;
- fDocumentClone= null;
- fOriginalContent= null;
- fOriginalLineDelimiters= null;
+ fActiveDocument= fDocument;
fireTextSet();
}
@Override
public void stopForwardingDocumentChanges() {
- fDocumentClone= null;
- fOriginalContent= fDocument.get();
- fOriginalLineDelimiters= fDocument.getLegalLineDelimiters();
fIsForwarding= false;
}
}
diff --git a/org.eclipse.jface.text/src/org/eclipse/jface/text/DocumentClone.java b/org.eclipse.jface.text/src/org/eclipse/jface/text/DocumentClone.java
index 8449330..5dc45ba 100644
--- a/org.eclipse.jface.text/src/org/eclipse/jface/text/DocumentClone.java
+++ b/org.eclipse.jface.text/src/org/eclipse/jface/text/DocumentClone.java
@@ -1,5 +1,5 @@
/*******************************************************************************
- * Copyright (c) 2000, 2007 IBM Corporation and others.
+ * Copyright (c) 2000, 2019 IBM Corporation and others.
*
* This program and the accompanying materials
* are made available under the terms of the Eclipse Public License 2.0
@@ -10,9 +10,12 @@
*
* Contributors:
* IBM Corporation - initial API and implementation
+ * Paul Pazderski - Bug 545252: cloned document was only partial read-only
*******************************************************************************/
package org.eclipse.jface.text;
+import java.util.Arrays;
+
import org.eclipse.core.runtime.Assert;
@@ -54,12 +57,13 @@
@Override
public void replace(int offset, int length, String text) {
+ // not allowed
}
@Override
public void set(String text) {
+ // not allowed
}
-
}
/**
@@ -72,20 +76,30 @@
super();
setTextStore(new StringTextStore(content));
- boolean hasDefaultDelims= lineDelimiters == null;
- if (!hasDefaultDelims && DefaultLineTracker.DELIMITERS.length == lineDelimiters.length) {
- hasDefaultDelims= true;
- for (int i= 0; i < lineDelimiters.length; i++) {
- if (DefaultLineTracker.DELIMITERS[i] != lineDelimiters[i]) {
- hasDefaultDelims= false;
- break;
- }
- }
- }
-
+ boolean hasDefaultDelims= Arrays.equals(lineDelimiters, DefaultLineTracker.DELIMITERS);
ILineTracker tracker= hasDefaultDelims ? new DefaultLineTracker() : new ConfigurableLineTracker(lineDelimiters);
setLineTracker(tracker);
getTracker().set(content);
completeInitialization();
}
+
+ @Override
+ public void replace(int pos, int length, String text) throws BadLocationException {
+ // not allowed
+ }
+
+ @Override
+ public void replace(int pos, int length, String text, long modificationStamp) throws BadLocationException {
+ // not allowed
+ }
+
+ @Override
+ public void set(String text) {
+ // not allowed
+ }
+
+ @Override
+ public void set(String text, long modificationStamp) {
+ //not allowed
+ }
}
