Esanders has uploaded a new change for review.
https://gerrit.wikimedia.org/r/60932
Change subject: Further AnnotationSet optimisation: create containsIndex
......................................................................
Further AnnotationSet optimisation: create containsIndex
In most places we call .contains we already know the index, so we
can avoid store lookups by using .containsIndex.
Change-Id: I45a9a421473f9bec479ab8ccceceb162b7004c3a
---
M modules/ve/dm/ve.dm.AnnotationSet.js
M modules/ve/dm/ve.dm.Converter.js
M modules/ve/dm/ve.dm.Transaction.js
M modules/ve/test/dm/ve.dm.AnnotationSet.test.js
4 files changed, 32 insertions(+), 11 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/VisualEditor
refs/changes/32/60932/1
diff --git a/modules/ve/dm/ve.dm.AnnotationSet.js
b/modules/ve/dm/ve.dm.AnnotationSet.js
index 0f2adc6..929c0fd 100644
--- a/modules/ve/dm/ve.dm.AnnotationSet.js
+++ b/modules/ve/dm/ve.dm.AnnotationSet.js
@@ -131,6 +131,17 @@
};
/**
+ * Check whether a given store index occurs in the set.
+ *
+ * @method
+ * @param {number} annotation Index of annotation in the store
+ * @returns {boolean} There is an annotation in the set with this store index
+ */
+ve.dm.AnnotationSet.prototype.containsIndex = function ( index ) {
+ return ve.indexOf( index , this.getIndexes() ) !== -1;
+};
+
+/**
* Check whether the set contains any of the annotations in another set.
*
* @method
@@ -249,7 +260,7 @@
return;
}
// if not in set already, splice in place
- if ( !this.contains( annotation ) ) {
+ if ( !this.containsIndex( storeIndex ) ) {
this.storeIndexes.splice( index, 0, storeIndex );
}
};
@@ -276,7 +287,7 @@
*/
ve.dm.AnnotationSet.prototype.push = function ( annotation ) {
var storeIndex = this.getStore().index( annotation );
- if ( !this.contains( annotation ) ) {
+ if ( !this.containsIndex( storeIndex ) ) {
this.storeIndexes.push( storeIndex );
}
};
diff --git a/modules/ve/dm/ve.dm.Converter.js b/modules/ve/dm/ve.dm.Converter.js
index 4db4fe9..524c057 100644
--- a/modules/ve/dm/ve.dm.Converter.js
+++ b/modules/ve/dm/ve.dm.Converter.js
@@ -66,14 +66,14 @@
* @param {Function} close Callback called when an annotation is closed.
Passed a ve.dm.Annotation.
*/
ve.dm.Converter.openAndCloseAnnotations = function ( currentSet, targetSet,
open, close ) {
- var i, len, arr, annotation, startClosingAt;
+ var i, len, arr, annotation, annotationIndex, startClosingAt;
// Close annotations as needed
// Go through annotationStack from bottom to top (low to high),
// and find the first annotation that's not in annotations.
- arr = currentSet.get();
+ arr = currentSet.getIndexes();
for ( i = 0, len = arr.length; i < len; i++ ) {
- annotation = arr[i];
- if ( !targetSet.contains( annotation ) ) {
+ annotationIndex = arr[i];
+ if ( !targetSet.containsIndex( annotationIndex ) ) {
startClosingAt = i;
break;
}
@@ -89,10 +89,11 @@
}
// Open annotations as needed
- arr = targetSet.get();
+ arr = targetSet.getIndexes();
for ( i = 0, len = arr.length; i < len; i++ ) {
- annotation = arr[i];
- if ( !currentSet.contains( annotation ) ) {
+ annotationIndex = arr[i];
+ if ( !currentSet.containsIndex( annotationIndex ) ) {
+ annotation = targetSet.getStore().value(
annotationIndex );
open( annotation );
// Add to currentClone
currentSet.push( annotation );
diff --git a/modules/ve/dm/ve.dm.Transaction.js
b/modules/ve/dm/ve.dm.Transaction.js
index 5adcd0d..b2a9a7d 100644
--- a/modules/ve/dm/ve.dm.Transaction.js
+++ b/modules/ve/dm/ve.dm.Transaction.js
@@ -199,7 +199,7 @@
* @param {string} method Annotation mode
* - `set`: Adds annotation to all content in range
* - `clear`: Removes instances of annotation from content in range
- * @param {Object} annotation Annotation to set or clear
+ * @param {ve.dm.Annotation} annotation Annotation to set or clear
* @returns {ve.dm.Transaction} Transaction that annotates content
*/
ve.dm.Transaction.newFromAnnotation = function ( doc, range, method,
annotation ) {
diff --git a/modules/ve/test/dm/ve.dm.AnnotationSet.test.js
b/modules/ve/test/dm/ve.dm.AnnotationSet.test.js
index 06096a1..7bc1684 100644
--- a/modules/ve/test/dm/ve.dm.AnnotationSet.test.js
+++ b/modules/ve/test/dm/ve.dm.AnnotationSet.test.js
@@ -9,7 +9,7 @@
/* Tests */
-QUnit.test( 'Basic usage', 28, function ( assert ) {
+QUnit.test( 'Basic usage', 33, function ( assert ) {
var annotationSet3,
store = new ve.dm.IndexValueStore(),
bold = new ve.dm.TextStyleBoldAnnotation(),
@@ -24,6 +24,8 @@
assert.deepEqual( annotationSet.get( 0 ), bold, 'get(0) is bold' );
assert.equal( annotationSet.contains( italic ), true, 'contains italic'
);
assert.equal( annotationSet.contains( underline ), false, 'doesn\'t
contain underline' );
+ assert.equal( annotationSet.containsIndex( 1 ), true, 'contains italic
by index' );
+ assert.equal( annotationSet.containsIndex( 2 ), false, 'doesn\'t
contain underline by index' );
assert.equal( annotationSet.containsAnyOf( annotationSet2 ), true,
'containsAnyOf set2 is true' );
assert.equal( annotationSet.containsAnyOf( emptySet ), false,
'containsAnyOf empty set is false' );
assert.equal( annotationSet.containsAllOf( annotationSet2 ), false,
'containsAllOf set2 set is false' );
@@ -43,6 +45,8 @@
assert.equal( annotationSet2.contains( bold ), false, 'set2 doesn\'t
contain bold after remove' );
annotationSet2.add( bold, 0 );
assert.equal( annotationSet2.indexOf( bold ), 0, 'set2 contains bold at
0 after add at 0' );
+ annotationSet2.add( bold, 0 );
+ assert.equal( annotationSet2.getLength(), 3, 'adding existing
annotation doesn\'t change length' );
// set is now [ bold, italic, underline ]
annotationSet2.removeAt( 2 );
assert.equal( annotationSet2.contains( underline ), false, 'set2
doesn\'t contain underline after removeAt 2' );
@@ -52,6 +56,11 @@
assert.equal( annotationSet.getLength(), 2, 'set2 has length 2 after
addSet' );
annotationSet2.removeSet( annotationSet );
assert.equal( annotationSet2.isEmpty(), true, 'set2 is empty after
removeSet' );
+ annotationSet2.push( bold );
+ annotationSet2.push( italic );
+ assert.deepEqual( annotationSet2.get(), [bold, italic], 'set2 contains
bold then italic after two pushes' );
+ annotationSet2.push( italic );
+ assert.deepEqual( annotationSet2.getLength(), 2, 'pushing existing
annotation doesn\'t change length' );
annotationSet2 = new ve.dm.AnnotationSet( store, store.indexes( [
italic, underline ] ) );
annotationSet2.removeNotInSet( annotationSet );
--
To view, visit https://gerrit.wikimedia.org/r/60932
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I45a9a421473f9bec479ab8ccceceb162b7004c3a
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/VisualEditor
Gerrit-Branch: master
Gerrit-Owner: Esanders <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
