perf: 60ms improvement: Replace fs-extra with mkdirp/rimraf/cp-file

In Electron apps, the cost of large modules is very real. fs-extra is very convenient, but removing it caused 50 fewer unique files to be required(), resultin in 60ms faster startup! Before: 557 unique requires (1330-1340ms) After: 507 unique requires (1270-1280ms)
2016-09-30 19:51:35 -07:00
parent d4cfc32c8d
commit 02f5dbb63f
8 changed files with 66 additions and 58 deletions
--- a/package.json
+++ b/package.json
@@ -18,6 +18,7 @@
    "bitfield": "^1.0.2",
    "capture-frame": "^1.0.0",
    "chromecasts": "^1.8.0",
+    "cp-file": "^3.2.0",
    "create-torrent": "^3.24.5",
    "debounce": "^1.0.0",
    "deep-equal": "^1.0.1",
@@ -25,11 +26,11 @@
    "drag-drop": "^2.12.1",
    "electron": "1.4.2",
    "es6-error": "^3.0.1",
-    "fs-extra": "^0.30.0",
    "iso-639-1": "^1.2.1",
    "languagedetect": "^1.1.1",
    "location-history": "^1.0.0",
    "material-ui": "^0.15.4",
+    "mkdirp": "^0.5.1",
    "musicmetadata": "^2.0.2",
    "network-address": "^1.1.0",
    "parse-torrent": "^5.7.3",
@@ -37,6 +38,7 @@
    "react": "^15.2.1",
    "react-dom": "^15.2.1",
    "react-tap-event-plugin": "^1.0.0",
+    "rimraf": "^2.5.2",
    "run-parallel": "^1.1.6",
    "semver": "^5.1.0",
    "simple-concat": "^1.0.0",
@@ -56,13 +58,11 @@
    "electron-winstaller": "^2.3.0",
    "gh-release": "^2.0.3",
    "minimist": "^1.2.0",
-    "mkdirp": "^0.5.1",
    "nobin-debian-installer": "^0.0.10",
    "nodemon": "^1.10.2",
    "open": "0.0.5",
    "plist": "^2.0.1",
    "pngjs": "^3.0.0",
-    "rimraf": "^2.5.2",
    "run-series": "^1.1.4",
    "spectron": "^3.3.0",
    "standard": "*",
--- a/src/main/handlers.js
+++ b/src/main/handlers.js
@@ -273,7 +273,7 @@ function commandToArgs (command) {
 }

 function installLinux () {
-  const fs = require('fs-extra')
+  const fs = require('fs')
  const os = require('os')
  const path = require('path')

@@ -326,6 +326,8 @@ function installLinux () {
  function writeIconFile (err, iconFile) {
    if (err) return log.error(err.message)

+    const mkdirp = require('mkdirp')
+
    const iconFilePath = path.join(
      os.homedir(),
      '.local',
@@ -333,9 +335,11 @@ function installLinux () {
      'icons',
      'webtorrent-desktop.png'
    )
-    fs.mkdirp(path.dirname(iconFilePath))
-    fs.writeFile(iconFilePath, iconFile, function (err) {
+    mkdirp(path.dirname(iconFilePath), (err) => {
      if (err) return log.error(err.message)
+      fs.writeFile(iconFilePath, iconFile, (err) => {
+        if (err) log.error(err.message)
+      })
    })
  }
 }
@@ -343,7 +347,7 @@ function installLinux () {
 function uninstallLinux () {
  const os = require('os')
  const path = require('path')
-  const fs = require('fs-extra')
+  const rimraf = require('rimraf')

  const desktopFilePath = path.join(
    os.homedir(),
@@ -352,7 +356,7 @@ function uninstallLinux () {
    'applications',
    'webtorrent-desktop.desktop'
  )
-  fs.removeSync(desktopFilePath)
+  rimraf(desktopFilePath)

  const iconFilePath = path.join(
    os.homedir(),
@@ -361,5 +365,5 @@ function uninstallLinux () {
    'icons',
    'webtorrent-desktop.png'
  )
-  fs.removeSync(iconFilePath)
+  rimraf(iconFilePath)
 }
--- a/src/renderer/controllers/subtitles-controller.js
+++ b/src/renderer/controllers/subtitles-controller.js
@@ -1,5 +1,5 @@
 const electron = require('electron')
-const fs = require('fs-extra')
+const fs = require('fs')
 const path = require('path')
 const parallel = require('run-parallel')

--- a/src/renderer/lib/migrations.js
+++ b/src/renderer/lib/migrations.js
@@ -48,7 +48,7 @@ function run (state) {
 }

 function migrate_0_7_0 (saved) {
-  const fs = require('fs-extra')
+  const cpFile = require('cp-file')
  const path = require('path')

  saved.torrents.forEach(function (ts) {
@@ -70,7 +70,7 @@ function migrate_0_7_0 (saved) {
      dst = path.join(config.TORRENT_PATH, infoHash + '.torrent')
      // Synchronous FS calls aren't ideal, but probably OK in a migration
      // that only runs once
-      if (src !== dst) fs.copySync(src, dst)
+      if (src !== dst) cpFile.sync(src, dst)

      delete ts.torrentPath
      ts.torrentFileName = infoHash + '.torrent'
@@ -85,7 +85,7 @@ function migrate_0_7_0 (saved) {
      dst = path.join(config.POSTER_PATH, infoHash + extension)
      // Synchronous FS calls aren't ideal, but probably OK in a migration
      // that only runs once
-      if (src !== dst) fs.copySync(src, dst)
+      if (src !== dst) cpFile.sync(src, dst)

      delete ts.posterURL
      ts.posterFileName = infoHash + extension
@@ -139,7 +139,7 @@ function migrate_0_12_0 (saved) {
    if (!fileOrFolder) return
    try {
      fs.statSync(fileOrFolder)
-    } catch (e) {
+    } catch (err) {
      // Default torrent with "missing path" error. Clear path.
      delete torrentSummary.path
    }
--- a/src/renderer/lib/state.js
+++ b/src/renderer/lib/state.js
@@ -109,7 +109,8 @@ function getDefaultPlayState () {

 /* If the saved state file doesn't exist yet, here's what we use instead */
 function setupStateSaved (cb) {
-  const fs = require('fs-extra')
+  const cpFile = require('cp-file')
+  const fs = require('fs')
  const parseTorrent = require('parse-torrent')
  const parallel = require('run-parallel')

@@ -130,18 +131,16 @@ function setupStateSaved (cb) {
  config.DEFAULT_TORRENTS.map(function (t, i) {
    const infoHash = saved.torrents[i].infoHash
    tasks.push(function (cb) {
-      fs.copy(
+      cpFile(
        path.join(config.STATIC_PATH, t.posterFileName),
-        path.join(config.POSTER_PATH, infoHash + path.extname(t.posterFileName)),
-        cb
-      )
+        path.join(config.POSTER_PATH, infoHash + path.extname(t.posterFileName))
+      ).then(cb).catch(cb)
    })
    tasks.push(function (cb) {
-      fs.copy(
+      cpFile(
        path.join(config.STATIC_PATH, t.torrentFileName),
-        path.join(config.TORRENT_PATH, infoHash + '.torrent'),
-        cb
-      )
+        path.join(config.TORRENT_PATH, infoHash + '.torrent')
+      ).then(cb).catch(cb)
    })
  })

@@ -151,6 +150,7 @@ function setupStateSaved (cb) {
  })

  function createTorrentObject (t) {
+    // TODO: Doing several fs.readFileSync calls during first startup is not ideal
    const torrent = fs.readFileSync(path.join(config.STATIC_PATH, t.torrentFileName))
    const parsedTorrent = parseTorrent(torrent)

--- a/src/renderer/webtorrent.js
+++ b/src/renderer/webtorrent.js
@@ -6,7 +6,8 @@ const crypto = require('crypto')
 const deepEqual = require('deep-equal')
 const defaultAnnounceList = require('create-torrent').announceList
 const electron = require('electron')
-const fs = require('fs-extra')
+const fs = require('fs')
+const mkdirp = require('mkdirp')
 const musicmetadata = require('musicmetadata')
 const networkAddress = require('network-address')
 const path = require('path')
@@ -203,19 +204,22 @@ function getTorrentFileInfo (file) {
  }
 }

-// Every time we resolve a magnet URI, save the torrent file so that we never
-// have to download it again. Never ask the DHT the same question twice.
+// Every time we resolve a magnet URI, save the torrent file so that we can use
+// it on next startup. Starting with the full torrent metadata will be faster
+// than re-fetching it from peers using ut_metadata.
 function saveTorrentFile (torrentKey) {
  const torrent = getTorrent(torrentKey)
-  checkIfTorrentFileExists(torrent.infoHash, function (torrentPath, exists) {
+  const torrentPath = path.join(config.TORRENT_PATH, torrent.infoHash + '.torrent')
+
+  fs.access(torrentPath, fs.constants.R_OK, function (err) {
    const fileName = torrent.infoHash + '.torrent'
-    if (exists) {
+    if (!err) {
      // We've already saved the file
      return ipc.send('wt-file-saved', torrentKey, fileName)
    }

    // Otherwise, save the .torrent file, under the app config folder
-    fs.mkdir(config.TORRENT_PATH, function (_) {
+    mkdirp(config.TORRENT_PATH, function (_) {
      fs.writeFile(torrentPath, torrent.torrentFile, function (err) {
        if (err) return console.log('error saving torrent file %s: %o', torrentPath, err)
        console.log('saved torrent file %s', torrentPath)
@@ -225,15 +229,6 @@ function saveTorrentFile (torrentKey) {
  })
 }

-// Checks whether we've already resolved a given infohash to a torrent file
-// Calls back with (torrentPath, exists). Logs, does not call back on error
-function checkIfTorrentFileExists (infoHash, cb) {
-  const torrentPath = path.join(config.TORRENT_PATH, infoHash + '.torrent')
-  fs.exists(torrentPath, function (exists) {
-    cb(torrentPath, exists)
-  })
-}
-
 // Save a JPG that represents a torrent.
 // Auto chooses either a frame from a video file, an image, etc
 function generateTorrentPoster (torrentKey) {
@@ -241,7 +236,7 @@ function generateTorrentPoster (torrentKey) {
  torrentPoster(torrent, function (err, buf, extension) {
    if (err) return console.log('error generating poster: %o', err)
    // save it for next time
-    fs.mkdirp(config.POSTER_PATH, function (err) {
+    mkdirp(config.POSTER_PATH, function (err) {
      if (err) return console.log('error creating poster dir: %o', err)
      const posterFileName = torrent.infoHash + extension
      const posterFilePath = path.join(config.POSTER_PATH, posterFileName)
--- a/test/setup.js
+++ b/test/setup.js
@@ -1,8 +1,11 @@
-const path = require('path')
 const Application = require('spectron').Application
-const fs = require('fs-extra')
+const fs = require('fs')
+const mkdirp = require('mkdirp')
 const parseTorrent = require('parse-torrent')
+const path = require('path')
 const PNG = require('pngjs').PNG
+const rimraf = require('rimraf')
+
 const config = require('./config')

 module.exports = {
@@ -75,8 +78,13 @@ function endTest (app, t, err) {
 function screenshotCreateOrCompare (app, t, name) {
  const ssDir = path.join(__dirname, 'screenshots', process.platform)
  const ssPath = path.join(ssDir, name + '.png')
-  fs.ensureFileSync(ssPath)
-  const ssBuf = fs.readFileSync(ssPath)
+  let ssBuf
+
+  try {
+    ssBuf = fs.readFileSync(ssPath)
+  } catch (err) {
+    ssBuf = Buffer.alloc(0)
+  }
  return wait().then(function () {
    return app.browserWindow.capturePage()
  }).then(function (buffer) {
@@ -136,14 +144,14 @@ function compareIgnoringTransparency (bufActual, bufExpected) {

 // Resets the test directory, containing config.json, torrents, downloads, etc
 function resetTestDataDir () {
-  fs.removeSync(config.TEST_DIR)
+  rimraf.sync(config.TEST_DIR)
  // Create TEST_DIR as well as /Downloads and /Desktop
-  fs.mkdirpSync(config.TEST_DIR_DOWNLOAD)
-  fs.mkdirpSync(config.TEST_DIR_DESKTOP)
+  mkdirp.sync(config.TEST_DIR_DOWNLOAD)
+  mkdirp.sync(config.TEST_DIR_DESKTOP)
 }

 function deleteTestDataDir () {
-  fs.removeSync(config.TEST_DIR)
+  rimraf.sync(config.TEST_DIR)
 }

 // Checks a given folder under Downloads.
@@ -159,11 +167,11 @@ function compareDownloadFolder (t, dirname, filenames) {
    const expectedSorted = filenames.slice().sort()
    const actualSorted = actualFilenames.slice().sort()
    t.deepEqual(actualSorted, expectedSorted, 'download folder contents: ' + dirname)
-  } catch (e) {
-    if (e.code === 'ENOENT') {
+  } catch (err) {
+    if (err.code === 'ENOENT') {
      t.equal(filenames, null, 'download folder missing: ' + dirname)
    } else {
-      console.error(e)
+      console.error(err)
      t.fail('unexpected error getting download folder: ' + dirname)
    }
  }
@@ -200,14 +208,14 @@ function extractImportantFields (parsedTorrent) {
 }

 function copy (pathFrom, pathTo) {
+  const cpFile = require('cp-file')
  try {
-    fs.copySync(pathFrom, pathTo)
-  } catch (e) {
-    // There is a bug in either node or `fs-extra` on windows
+    cpFile.sync(pathFrom, pathTo)
+  } catch (err) {
    // Windows lets us create files and folders under C:\Windows\Temp,
    // but when you try to `copySync` into one of those folders, you get EPERM
    // Ignore for now...
-    if (process.platform !== 'win32' || e.code !== 'EPERM') throw e
-    console.log('ignoring windows copy EPERM error', e)
+    if (process.platform !== 'win32' || err.code !== 'EPERM') throw err
+    console.log('ignoring windows copy EPERM error', err)
  }
 }
--- a/test/test-torrent-list.js
+++ b/test/test-torrent-list.js
@@ -1,11 +1,12 @@
+const rimraf = require('rimraf')
 const test = require('tape')
-const fs = require('fs-extra')
-const setup = require('./setup')
+
 const config = require('./config')
+const setup = require('./setup')

 test('torrent-list: show download path missing', function (t) {
  setup.resetTestDataDir()
-  fs.removeSync(config.TEST_DIR_DOWNLOAD)
+  rimraf.sync(config.TEST_DIR_DOWNLOAD)

  t.timeoutAfter(20e3)
  const app = setup.createApp()