From 5b38dc0b1097a85b2e4ddb80035ed7e4d1533247 Mon Sep 17 00:00:00 2001 From: jfstephe Date: Mon, 15 Nov 2021 09:24:19 +0000 Subject: [PATCH] perf(watcher): significantly improved addFile/changeFile/removeFile perf Change to the event handlers so that they don't return the, generally unecessary, list of files which can save around 100ms per-event (depending on machine spec) BREAKING CHANGE: Core public API change: Handlers no longer return the list of files, and a separate call is now needed to return that information. --- lib/file-list.js | 13 +++++-------- test/unit/file-list.spec.js | 28 ++++++++++++++-------------- 2 files changed, 19 insertions(+), 22 deletions(-) diff --git a/lib/file-list.js b/lib/file-list.js index 6c9e93d16..16feb1927 100644 --- a/lib/file-list.js +++ b/lib/file-list.js @@ -114,6 +114,7 @@ class FileList { return lastCompletedRefresh } + // TODO - Change this so that it's not a getter. Getters shouldn't do lots of processing! get files () { const served = [] const included = {} @@ -172,18 +173,18 @@ class FileList { const excluded = this._findExcluded(path) if (excluded) { log.debug(`Add file "${path}" ignored. Excluded by "${excluded}".`) - return this.files + return } const pattern = this._findIncluded(path) if (!pattern) { log.debug(`Add file "${path}" ignored. Does not match any pattern.`) - return this.files + return } if (this._exists(path)) { log.debug(`Add file "${path}" ignored. Already in the list.`) - return this.files + return } const file = new File(path) @@ -195,7 +196,6 @@ class FileList { log.info(`Added file "${path}".`) this._emitModified() - return this.files } async changeFile (path, force) { @@ -204,7 +204,7 @@ class FileList { if (!file) { log.debug(`Changed file "${path}" ignored. Does not match any file in the list.`) - return this.files + return } const [stat] = await Promise.all([statAsync(path), this._refreshing]) @@ -214,7 +214,6 @@ class FileList { log.info(`Changed file "${path}".`) this._emitModified(force) } - return this.files } async removeFile (path) { @@ -224,12 +223,10 @@ class FileList { if (file) { helper.arrayRemove(this._getFilesByPattern(pattern.pattern), file) log.info(`Removed file "${path}".`) - this._emitModified() } else { log.debug(`Removed file "${path}" ignored. Does not match any file in the list.`) } - return this.files } } diff --git a/test/unit/file-list.spec.js b/test/unit/file-list.spec.js index ca2d39fee..89166b5bd 100644 --- a/test/unit/file-list.spec.js +++ b/test/unit/file-list.spec.js @@ -464,32 +464,32 @@ describe('FileList', () => { it('does not add excluded files', () => { return list.refresh().then((before) => { - return list.addFile('/secret/hello.txt').then((files) => { - expect(files.served).to.be.eql(before.served) + return list.addFile('/secret/hello.txt').then(() => { + expect(list.files.served).to.be.eql(before.served) }) }) }) it('does not add already existing files', () => { return list.refresh().then((before) => { - return list.addFile('/some/a.js').then((files) => { - expect(files.served).to.be.eql(before.served) + return list.addFile('/some/a.js').then(() => { + expect(list.files.served).to.be.eql(before.served) }) }) }) it('does not add unmatching files', () => { return list.refresh().then((before) => { - return list.addFile('/some/a.ex').then((files) => { - expect(files.served).to.be.eql(before.served) + return list.addFile('/some/a.ex').then(() => { + expect(list.files.served).to.be.eql(before.served) }) }) }) it('adds the file to the correct bucket', () => { return list.refresh().then((before) => { - return list.addFile('/some/d.js').then((files) => { - expect(pathsFrom(files.served)).to.contain('/some/d.js') + return list.addFile('/some/d.js').then(() => { + expect(pathsFrom(list.files.served)).to.contain('/some/d.js') const bucket = list.buckets.get('/some/*.js') expect(pathsFrom(bucket)).to.contain('/some/d.js') }) @@ -531,8 +531,8 @@ describe('FileList', () => { list = new List(patterns('/a.*'), [], emitter, preprocess) return list.refresh().then(() => { - return list.addFile('/a.js').then((files) => { - expect(findFile('/a.js', files.served).mtime).to.eql(new Date('2012-01-01')) + return list.addFile('/a.js').then(() => { + expect(findFile('/a.js', list.files.served).mtime).to.eql(new Date('2012-01-01')) }) }) }) @@ -597,10 +597,10 @@ describe('FileList', () => { mockFs._touchFile('/some/b.js', '3020-01-01') modified.resetHistory() - return list.changeFile('/some/b.js').then((files) => { + return list.changeFile('/some/b.js').then(() => { clock.tick(101) expect(modified).to.have.been.calledOnce - expect(findFile('/some/b.js', files.served).mtime).to.be.eql(new Date('3020-01-01')) + expect(findFile('/some/b.js', list.files.served).mtime).to.be.eql(new Date('3020-01-01')) }) }) }) @@ -719,8 +719,8 @@ describe('FileList', () => { return list.refresh().then((files) => { modified.resetHistory() return list.removeFile('/some/a.js') - }).then((files) => { - expect(pathsFrom(files.served)).to.be.eql([ + }).then(() => { + expect(pathsFrom(list.files.served)).to.be.eql([ '/some/b.js', '/a.txt' ])