diff --git a/services/document-updater/app/js/sharejs/types/text.js b/services/document-updater/app/js/sharejs/types/text.js index d733837b34..bbbe36e0f6 100644 --- a/services/document-updater/app/js/sharejs/types/text.js +++ b/services/document-updater/app/js/sharejs/types/text.js @@ -32,8 +32,6 @@ // NOTE: The global scope here is shared with other sharejs files when built with closure. // Be careful what ends up in your namespace. -const Rope = require('jumprope') - let append, transformComponent const text = {} @@ -70,23 +68,28 @@ const checkValidOp = function (op) { } text.apply = function (snapshot, op) { - const rope = new Rope(snapshot) checkValidOp(op) for (const component of Array.from(op)) { if (component.i != null) { - rope.insert(component.p, component.i) + snapshot = strInject(snapshot, component.p, component.i) } else if (component.d != null) { - const deleted = ropeSubstring(rope, component.p, component.d.length) + const deleted = snapshot.slice( + component.p, + component.p + component.d.length + ) if (component.d !== deleted) { throw new Error( `Delete component '${component.d}' does not match deleted text '${deleted}'` ) } - rope.del(component.p, component.d.length) + snapshot = + snapshot.slice(0, component.p) + + snapshot.slice(component.p + component.d.length) } else if (component.c != null) { - // The rope has strict bounds checks, so we need to make sure we don't - // extract text beyond the end of the rope - const comment = ropeSubstring(rope, component.p, component.c.length) + const comment = snapshot.slice( + component.p, + component.p + component.c.length + ) if (component.c !== comment) { throw new Error( `Comment component '${component.c}' does not match commented text '${comment}'` @@ -96,7 +99,7 @@ text.apply = function (snapshot, op) { throw new Error('Unknown op type') } } - return rope.toString() + return snapshot } // Exported for use by the random op generator. @@ -385,11 +388,3 @@ if (typeof WEB !== 'undefined' && WEB !== null) { append ) } - -function ropeSubstring(rope, start, length) { - // The rope has strict bounds checks, so we need to make sure we don't - // extract text beyond the end of the rope. - start = Math.min(rope.length, start) - length = Math.min(rope.length - start, length) - return rope.substring(start, length) -} diff --git a/services/document-updater/benchmarks/apply_string_rope.js b/services/document-updater/benchmarks/apply_string_rope.js deleted file mode 100644 index a1fb5b4ed7..0000000000 --- a/services/document-updater/benchmarks/apply_string_rope.js +++ /dev/null @@ -1,43 +0,0 @@ -const { Suite } = require('benchmark') -const Rope = require('jumprope') - -const suite = new Suite() - -for (const size of [5000, 100000, 2000000]) { - const positions = [] - for (let i = 0; i < 100; i++) { - positions.push(Math.floor(Math.random() * size)) - } - for (const inserts of [0, 1, 2, 5, 10, 20, 100, 1000, 10000]) { - const initialString = 'a'.repeat(size) - suite.add(`| string | ${inserts} | ${size} |`, () => { - let str = initialString - for (let i = 0; i < inserts; i++) { - const pos = positions[i % 100] - str = str.slice(0, pos) + '1234567890' + str.slice(pos) - } - return str.length - }) - - suite.add(`| rope | ${inserts} | ${size} |`, () => { - const rope = new Rope(initialString) - for (let i = 0; i < inserts; i++) { - const pos = positions[i % 100] - rope.insert(pos, '1234567890') - } - return rope.toString().length - }) - } -} - -suite.on('cycle', function (event) { - const meanMs = event.target.stats.mean * 1000 - const meanToString = Math.ceil(meanMs * 100000) / 100000 - console.log(`${event.target.name} ${meanToString}ms | `) -}) - -// markdown table headers -console.log('| method | inserts | string size | mean |') -console.log('|-|-|-|-|') - -suite.run({ async: true }) diff --git a/services/document-updater/package-lock.json b/services/document-updater/package-lock.json index 4e83c127a6..76950022d0 100644 --- a/services/document-updater/package-lock.json +++ b/services/document-updater/package-lock.json @@ -575,16 +575,6 @@ "tweetnacl": "^0.14.3" } }, - "benchmark": { - "version": "2.1.4", - "resolved": "https://registry.npmjs.org/benchmark/-/benchmark-2.1.4.tgz", - "integrity": "sha1-CfPeMckWQl1JjMLuVloOvzwqVik=", - "dev": true, - "requires": { - "lodash": "^4.17.4", - "platform": "^1.3.3" - } - }, "bignumber.js": { "version": "9.0.1", "resolved": "https://registry.npmjs.org/bignumber.js/-/bignumber.js-9.0.1.tgz", @@ -1732,10 +1722,6 @@ "verror": "1.10.0" } }, - "jumprope": { - "version": "https://github.com/overleaf/jumprope/archive/b3ce482e83925a6c23278f3230a47a82bd440592.tar.gz", - "integrity": "sha512-VSZr8UrKvY551MPwPR8lramBcCcnMDBh03qliCb1q2RaeQQR1GEsbJ5rAY1PCITtZXCOqeYgsEP0cSj2A0eY8w==" - }, "just-extend": { "version": "4.2.1", "resolved": "https://registry.npmjs.org/just-extend/-/just-extend-4.2.1.tgz", @@ -2284,12 +2270,6 @@ "resolved": "https://registry.npmjs.org/pify/-/pify-5.0.0.tgz", "integrity": "sha512-eW/gHNMlxdSP6dmG6uJip6FXN0EQBwm2clYYd8Wul42Cwu/DK8HEftzsapcNdYe2MfLiIwZqsDk2RDEsTE79hA==" }, - "platform": { - "version": "1.3.6", - "resolved": "https://registry.npmjs.org/platform/-/platform-1.3.6.tgz", - "integrity": "sha512-fnWVljUchTro6RiCFvCXBbNhJc2NijN7oIQxbwsyL0buWJPG85v81ehlHI9fXrJsMNgTofEoWIQeClKpgxFLrg==", - "dev": true - }, "pprof": { "version": "3.2.0", "resolved": "https://registry.npmjs.org/pprof/-/pprof-3.2.0.tgz", diff --git a/services/document-updater/package.json b/services/document-updater/package.json index 37daadc97c..068ede7efb 100644 --- a/services/document-updater/package.json +++ b/services/document-updater/package.json @@ -29,14 +29,12 @@ "bunyan": "^1.8.15", "diff-match-patch": "https://github.com/overleaf/diff-match-patch/archive/89805f9c671a77a263fc53461acd62aa7498f688.tar.gz", "express": "4.17.1", - "jumprope": "https://github.com/overleaf/jumprope/archive/b3ce482e83925a6c23278f3230a47a82bd440592.tar.gz", "lodash": "^4.17.21", "mongodb": "^3.6.6", "request": "^2.88.2", "requestretry": "^4.1.2" }, "devDependencies": { - "benchmark": "^2.1.4", "chai": "^4.2.0", "chai-as-promised": "^7.1.1", "cluster-key-slot": "^1.0.5", diff --git a/services/document-updater/test/unit/js/ShareJS/TextTransformTests.js b/services/document-updater/test/unit/js/ShareJS/TextTransformTests.js index 45662bcc26..118cf339c2 100644 --- a/services/document-updater/test/unit/js/ShareJS/TextTransformTests.js +++ b/services/document-updater/test/unit/js/ShareJS/TextTransformTests.js @@ -13,7 +13,6 @@ * DS205: Consider reworking code to avoid use of IIFEs * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md */ -const { expect } = require('chai') const text = require('../../../../app/js/sharejs/types/text') const RangesTracker = require('../../../../app/js/RangesTracker') @@ -269,84 +268,15 @@ describe('ShareJS text type', function () { .should.equal('foo123bar') }) - it('should apply delete operations that are not in order', function () { - const result = text.apply('0123456789', [ - { d: '9', p: 9 }, - { d: '4', p: 4 }, - { d: '7', p: 6 }, - ]) - expect(result).to.equal('0123568') - }) - - it('should apply a mix of inserts and deletes in any order', function () { - const result = text.apply('we can insert and delete in any order!', [ - { d: 'any ', p: 28 }, // we can insert and delete in order! - { i: 'not', p: 6 }, // we cannot insert and delete in order! - { d: 'insert and ', p: 10 }, // we cannot delete in order! - { i: 'the same ', p: 20 }, - ]) - expect(result).to.equal('we cannot delete in the same order!') - }) - - it('should apply a mix of inserts and deletes in order', function () { - const result = text.apply('we can insert and delete in any order!', [ - { i: 'not', p: 6 }, // we cannot insert and delete in any order! - { d: 'insert and ', p: 10 }, // we cannot delete in any order! - { d: 'any ', p: 20 }, // we cannot delete in order! - { i: 'the same ', p: 20 }, - ]) - expect(result).to.equal('we cannot delete in the same order!') - }) - - it('should be able to insert a string, then delete it along with some more text that comes before', function () { - const result = text.apply('I love cake and cookies', [ - { i: ', vegetables', p: 11 }, // I love cake, vegetables and cookies - { d: 'cake, vege', p: 7 }, - ]) - expect(result).to.equal('I love tables and cookies') - }) - - it('should be able to insert a string, then delete it along with some more text that comes after', function () { - const result = text.apply('I love cake and cookies', [ - { i: 'chocolate, ', p: 7 }, // I love chocolate, cake and cookies - { d: ', cake and', p: 16 }, - ]) - expect(result).to.equal('I love chocolate cookies') - }) - it('should throw an error when deleted content does not match', function () { - ;(() => text.apply('foo123bar', [{ d: '456', p: 3 }])).should.throw( - /^Delete component/ + return (() => text.apply('foo123bar', [{ d: '456', p: 3 }])).should.throw( + Error ) }) - it('should throw an error when deleted content extends beyond the text', function () { - ;(() => text.apply('foo123bar', [{ d: '123barbaz', p: 3 }])).should.throw( - /^Delete component/ - ) - }) - - it('should throw an error when deleted content is out of bounds', function () { - ;(() => text.apply('foo123bar', [{ d: '456', p: 20 }])).should.throw( - /^Delete component/ - ) - }) - - it('should throw an error when comment content does not match', function () { - ;(() => text.apply('foo123bar', [{ c: '456', p: 3 }])).should.throw( - /^Comment component/ - ) - }) - - it('should throw an error when commented content extends beyond the text', function () { - ;(() => text.apply('foo123bar', [{ c: '123barbaz', p: 3 }])).should.throw( - /^Comment component/ - ) - }) - - it('should throw an error when commented content is out of bounds', function () { - ;(() => text.apply('foo123bar', [{ c: '456', p: 20 }])).should.throw( - /^Comment component/ + return it('should throw an error when comment content does not match', function () { + return (() => text.apply('foo123bar', [{ c: '456', p: 3 }])).should.throw( + Error ) }) })