src,lib: expose memory file mapping flag by joaocgreis · Pull Request #29260 · nodejs/node

joaocgreis · 2019-08-22T00:23:35Z

Support for UV_FS_O_FILEMAP was added in libuv in version 1.31.0. This exposes the flag in fs.constants and adds the prefix 'm' for text flags.

New functionality is added, so this is semver minor.

cc @nodejs/platform-windows

Checklist

make -j4 test (UNIX), or vcbuild test (Windows) passes
tests and/or benchmarks are included
documentation is changed or added
commit message follows commit guidelines

Support for UV_FS_O_FILEMAP was added in libuv in version 1.31.0. This exposes the flag in fs.constants and adds the prefix 'm' for text flags.

Trott · 2019-08-22T00:27:21Z

@nodejs/fs @nodejs/libuv

nodejs-github-bot · 2019-08-22T00:28:00Z

CI: https://ci.nodejs.org/job/node-test-pull-request/25170/

doc/api/fs.md

bnoordhuis

LGTM with suggestions.

Question: is e.g. 'ma' (memory-mapped, append-only) a mode that makes sense?

src/node_constants.cc

test/parallel/test-fs-open-flags.js

Co-Authored-By: Rich Trott <rtrott@gmail.com>

joaocgreis · 2019-08-22T16:01:43Z

Thanks for the reviews! Updated.

Question: is e.g. 'ma' (memory-mapped, append-only) a mode that makes sense?

I tried to keep the libuv API as compatible as possible. This included supporting positional writes and keeping track of the file position. On top of this, supporting append makes sense to me, and it was not that complex:

node/deps/uv/src/win/fs.c

Lines 934 to 935 in 775048d

    
           if (force_append) { 
        
             pos = fd_info->size;

Making multiple appends to the same file is much slower than without using a file mapping though, because the mapping has to be re-created to enlarge the file every time.

bnoordhuis · 2019-08-24T17:24:17Z

I did some research and it seems mmap() on an O_APPEND file descriptor fails with EACCES on most Unices (which makes sense to me.)

I think we should revisit the libuv behavior and drop the 'ma' flags from this pull request.

joaocgreis · 2019-09-02T21:32:50Z

@bnoordhuis removed the 'm' flags from this PR. Opened libuv/libuv#2448 to discuss the libuv side.

bnoordhuis

LGTM. The 'mr' and 'mw' flags would still have been alright but those can always be added later.

doc/api/fs.md

joaocgreis · 2019-09-04T10:51:43Z

@jasnell updated, PTAL.

joaocgreis · 2019-09-09T17:24:23Z

@jasnell any objection for landing this?

nodejs-github-bot · 2019-09-11T22:29:31Z

CI: https://ci.nodejs.org/job/node-test-pull-request/25432/ ✔️

Support for UV_FS_O_FILEMAP was added in libuv in version 1.31.0. This exposes the flag in fs.constants. Refs: libuv/libuv#2295 PR-URL: #29260 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Ron Korving <ron@ronkorving.nl> Reviewed-By: James M Snell <jasnell@gmail.com>

joaocgreis · 2019-09-11T23:02:07Z

Landed in 902c9fa

Support for UV_FS_O_FILEMAP was added in libuv in version 1.31.0. This exposes the flag in fs.constants. Refs: libuv/libuv#2295 PR-URL: #29260 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Ron Korving <ron@ronkorving.nl> Reviewed-By: James M Snell <jasnell@gmail.com>

Notable changes: * crypto: * Add `oaepLabel` option #29489 * deps: * Update V8 to 7.7.299.11 #28918 * More efficient memory handling * Stack trace serialization got faster * The `Intl.NumberFormat` API gained new functionality * For more information: https://v8.dev/blog/v8-release-77 * events: * Add support for `EventTarget` in `once` #29498 * fs: * Expose memory file mapping flag `UV_FS_O_FILEMAP` #29260 * inspector: * New API - `Session.connectToMainThread` #28870 * process: * Initial SourceMap support via `env.NODE_V8_COVERAGE` #28960 * stream: * Make `_write()` optional when `_writev()` is implemented #29639 * tls: * Add option to override signature algorithms #29598 * util: * Add `encodeInto` to `TextEncoder` #29524 * worker: * The `worker_thread` module is now stable #29512

Support for UV_FS_O_FILEMAP was added in libuv in version 1.31.0. This exposes the flag in fs.constants. Refs: libuv/libuv#2295 PR-URL: nodejs#29260 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Ron Korving <ron@ronkorving.nl> Reviewed-By: James M Snell <jasnell@gmail.com>

Support for UV_FS_O_FILEMAP was added in libuv in version 1.31.0. This exposes the flag in fs.constants. Refs: libuv/libuv#2295 PR-URL: #29260 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Ron Korving <ron@ronkorving.nl> Reviewed-By: James M Snell <jasnell@gmail.com>

Notable changes: * crypto: * Add `oaepLabel` option #29489 * deps: * Update V8 to 7.7.299.11 #28918 * More efficient memory handling * Stack trace serialization got faster * The `Intl.NumberFormat` API gained new functionality * For more information: https://v8.dev/blog/v8-release-77 * events: * Add support for `EventTarget` in `once` #29498 * fs: * Expose memory file mapping flag `UV_FS_O_FILEMAP` #29260 * inspector: * New API - `Session.connectToMainThread` #28870 * process: * Initial SourceMap support via `env.NODE_V8_COVERAGE` #28960 * stream: * Make `_write()` optional when `_writev()` is implemented #29639 * tls: * Add option to override signature algorithms #29598 * util: * Add `encodeInto` to `TextEncoder` #29524 * worker: * The `worker_thread` module is now stable #29512 PR-URL: #29695

src,lib: expose memory file mapping flag

2002dd1

Support for UV_FS_O_FILEMAP was added in libuv in version 1.31.0. This exposes the flag in fs.constants and adds the prefix 'm' for text flags.

joaocgreis added the semver-minor PRs that contain new features and should be released in the next minor version. label Aug 22, 2019

nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. fs Issues and PRs related to the fs subsystem / file system. labels Aug 22, 2019

This comment has been minimized.

Sign in to view

mscdex reviewed Aug 22, 2019

View reviewed changes

doc/api/fs.md Outdated Show resolved Hide resolved

Trott reviewed Aug 22, 2019

View reviewed changes

doc/api/fs.md Outdated Show resolved Hide resolved

bnoordhuis approved these changes Aug 22, 2019

View reviewed changes

src/node_constants.cc Outdated Show resolved Hide resolved

test/parallel/test-fs-open-flags.js Show resolved Hide resolved

joaocgreis and others added 3 commits August 22, 2019 16:32

fixup! src,lib: expose memory file mapping flag

a7146a3

Co-Authored-By: Rich Trott <rtrott@gmail.com>

fixup! src,lib: expose memory file mapping flag

b447c3c

Co-Authored-By: Rich Trott <rtrott@gmail.com>

fixup! src,lib: expose memory file mapping flag

45cea0b

fixup! src,lib: expose memory file mapping flag

c77569e

joaocgreis mentioned this pull request Sep 2, 2019

win: fail fs_open with append and file mapping libuv/libuv#2448

Closed

bnoordhuis approved these changes Sep 3, 2019

View reviewed changes

jasnell reviewed Sep 3, 2019

View reviewed changes

doc/api/fs.md Outdated Show resolved Hide resolved

fixup! src,lib: expose memory file mapping flag

cc923d3

ronkorving approved these changes Sep 9, 2019

View reviewed changes

jasnell approved these changes Sep 11, 2019

View reviewed changes

joaocgreis closed this Sep 11, 2019

joaocgreis mentioned this pull request Sep 12, 2019

unpack: accept fflag as an option isaacs/node-tar#227

Closed

joaocgreis mentioned this pull request Sep 12, 2019

extract-stream: use file mapping on Windows npm/pacote#8

Closed

BridgeAR mentioned this pull request Sep 24, 2019

v12.11.0 proposal #29695

Merged

Uh oh!

Conversation

joaocgreis commented Aug 22, 2019

Checklist

Uh oh!

Trott commented Aug 22, 2019

Uh oh!

This comment has been minimized.

nodejs-github-bot commented Aug 22, 2019

Uh oh!

Uh oh!

Uh oh!

bnoordhuis left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

joaocgreis commented Aug 22, 2019

Uh oh!

bnoordhuis commented Aug 24, 2019

Uh oh!

joaocgreis commented Sep 2, 2019

Uh oh!

bnoordhuis left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

joaocgreis commented Sep 4, 2019

Uh oh!

joaocgreis commented Sep 9, 2019

Uh oh!

nodejs-github-bot commented Sep 11, 2019 • edited by joaocgreis Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

joaocgreis commented Sep 11, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

nodejs-github-bot commented Sep 11, 2019 •

edited by joaocgreis

Loading