Skip to content

Commit

Permalink
Extract getFlag fmap detection to a tested module
Browse files Browse the repository at this point in the history
  • Loading branch information
isaacs committed Sep 26, 2019
1 parent 150da0e commit 97f813a
Show file tree
Hide file tree
Showing 3 changed files with 68 additions and 8 deletions.
16 changes: 16 additions & 0 deletions lib/get-write-flag.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
// Get the appropriate flag to use for creating files
// We use fmap on Windows platforms for files less than
// 512kb. This is a fairly low limit, but avoids making
// things slower in some cases. Since most of what this
// library is used for is extracting tarballs of many
// relatively small files in npm packages and the like,
// it can be a big boost on Windows platforms.
// Only supported in Node v12.9.0 and above.

This comment has been minimized.

Copy link
@joaocgreis

joaocgreis Sep 26, 2019

Contributor

Actually, v12.11.0. The constant UV_FS_O_FILEMAP was not exposed in fs.constants before so this won't work.

const platform = process.env.__FAKE_PLATFORM__ || process.platform
const isWindows = platform === 'win32'
const fs = require('fs')
const { O_CREAT, O_TRUNC, O_WRONLY, UV_FS_O_FILEMAP = 0 } = fs.constants
const fMapEnabled = isWindows && !!UV_FS_O_FILEMAP
const fMapLimit = 512 * 1024
const fMapFlag = UV_FS_O_FILEMAP | O_TRUNC | O_CREAT | O_WRONLY
module.exports = size => (fMapEnabled && size < fMapLimit) ? fMapFlag : 'w'
9 changes: 1 addition & 8 deletions lib/unpack.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ const assert = require('assert')
const EE = require('events').EventEmitter
const Parser = require('./parse.js')
const fs = require('fs')
const { O_CREAT, O_TRUNC, O_WRONLY, UV_FS_O_FILEMAP = 0 } = fs.constants
const fsm = require('fs-minipass')
const path = require('path')
const mkdir = require('./mkdir.js')
Expand Down Expand Up @@ -43,6 +42,7 @@ const DOCHOWN = Symbol('doChown')
const UID = Symbol('uid')
const GID = Symbol('gid')
const crypto = require('crypto')
const getFlag = require('./get-write-flag.js')

/* istanbul ignore next */
const neverCalled = () => {
Expand Down Expand Up @@ -93,13 +93,6 @@ const uint32 = (a, b, c) =>
: b === b >>> 0 ? b
: c

/* istanbul ignore next */
const fMapEnabled = process.platform === 'win32' && !!UV_FS_O_FILEMAP
const fMapLimit = 512 * 1024
const fMapFlag = UV_FS_O_FILEMAP | O_TRUNC | O_CREAT | O_WRONLY
/* istanbul ignore next */
const getFlag = size => (fMapEnabled && size < fMapLimit) ? fMapFlag : 'w'

class Unpack extends Parser {
constructor (opt) {
if (!opt)
Expand Down
51 changes: 51 additions & 0 deletions test/get-write-flag.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
const t = require('tap')

// run three scenarios
// unix (no fmap)
// win32 (without fmap support)
// win32 (with fmap support)

const fs = require('fs')
const hasFmap = !!fs.constants.UV_FS_O_FILEMAP
const platform = process.platform

switch (process.argv[2]) {
case 'win32-fmap': {
if (!hasFmap)
fs.constants.UV_FS_O_FILEMAP = 0x20000000
const { O_CREAT, O_TRUNC, O_WRONLY, UV_FS_O_FILEMAP } = fs.constants
if (platform !== 'win32')
process.env.__FAKE_PLATFORM__ = 'win32'
const getFlag = require('../lib/get-write-flag.js')
t.equal(getFlag(1), UV_FS_O_FILEMAP | O_TRUNC | O_CREAT | O_WRONLY)
t.equal(getFlag(512 * 1024 + 1), 'w')
break
}

case 'win32-nofmap': {
if (hasFmap)
fs.constants.UV_FS_O_FILEMAP = 0
if (platform !== 'win32')
process.env.__FAKE_PLATFORM__ = 'win32'
const getFlag = require('../lib/get-write-flag.js')
t.equal(getFlag(1), 'w')
t.equal(getFlag(512 * 1024 + 1), 'w')
break
}

case 'unix': {
if (platform === 'win32')
process.env.__FAKE_PLATFORM__ = 'darwin'
const getFlag = require('../lib/get-write-flag.js')
t.equal(getFlag(1), 'w')
t.equal(getFlag(512 * 1024 + 1), 'w')
break
}

default: {
const node = process.execPath
t.spawn(node, [__filename, 'win32-fmap'])
t.spawn(node, [__filename, 'win32-nofmap'])
t.spawn(node, [__filename, 'unix'])
}
}

0 comments on commit 97f813a

Please sign in to comment.