Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for O_NOATIME flag to fs.open #2182

Closed
jorangreef opened this issue Jul 15, 2015 · 9 comments
Closed

Add support for O_NOATIME flag to fs.open #2182

jorangreef opened this issue Jul 15, 2015 · 9 comments
Labels
feature request Issues that request new features to be added to Node.js. fs Issues and PRs related to the fs subsystem / file system.

Comments

@jorangreef
Copy link
Contributor

I checked through the source but it doesn't look like O_NOATIME can be passed to fs.open?

fs.fstat on many files with node.js would be much faster if the fd could be opened with O_NOATIME.

For indexing or backup programs using node, O_NOATIME can significantly reduce the amount of disk activity.

@bnoordhuis
Copy link
Member

The reason it's not exposed is probably because it's a Linux-specific flag. You can use it, however:

const O_RDONLY = require('constants').O_RDONLY;
const O_NOATIME = 0x40000;
fs.open(filename, O_RDONLY | O_NOATIME, function(err, fd) { /* ... */ });

I wouldn't object to a pull request that adds it to src/node_constants.cc but I don't feel strongly about it either.

@meandmycode
Copy link

Seems like you can get the same effect in windows according to: http://blogs.msdn.com/b/oldnewthing/archive/2011/10/10/10222560.aspx

SetFileTime
FILETIME (documents the 0xFFFFFFFF value)

But I assume this would be a libuv feature request.

@jorangreef
Copy link
Contributor Author

Thanks Ben, I tried out the following over 10,000 files:

var O_RDONLY = require('constants').O_RDONLY;
var O_NOATIME = 0x40000;
fs.lstat2 = function(path, end) {
  fs.open(path, O_RDONLY | O_NOATIME,
    function(error, fd) {
      if (error) return end(error);
      fs.fstat(fd,
        function(fstatError, stats) {
          fs.close(fd,
            function(closeError) {
              if (fstatError) return end(fstatError);
              if (closeError) return end(closeError);
              end(undefined, stats);
            }
          );
        }
      );
    }
  );
};

This was actually slower than lstat (3000ms vs 1600ms on average over many runs), probably because of the multiple system calls, whereas I think lstat is a single binding call.

I think I was also mistaken, in that lstat doesn't seem to update the atime (at least not on Mac), although readdir and readFile do (this could possibly be a minor performance improvement to readFile's implementation).

Would the following be all that's needed in node_constants.cc to add O_NOATIME?

#ifdef O_NOATIME
  NODE_DEFINE_CONSTANT(target, O_NOATIME);
#endif

@brendanashworth brendanashworth added fs Issues and PRs related to the fs subsystem / file system. feature request Issues that request new features to be added to Node.js. labels Jul 15, 2015
@ronkorving
Copy link
Contributor

I would love to have access to this. In #2902 there has been discussion recently about whether or not using flags this way are an anti-pattern (as its undocumented), or if they should be documented. I think the above would add weight to allowing integers from userland and thus a need to document it and expose the constants perhaps in a more friendly way. cc @sam-github

@ChALkeR
Copy link
Member

ChALkeR commented Sep 18, 2015

@jorangreef Isn't it best to mount the whole fs with noatime? This shouldn't be a problem of the platform/application.

@ronkorving
Copy link
Contributor

@ChALkeR Isn't that assuming a lot about all possible applications? :)

@jorangreef
Copy link
Contributor Author

@ChALkeR You could do that and it's probably a good idea on a server but it's still a change at the filesystem level.

Applications should still be free to decide whether to update atime on a case-by-case basis, so if you were writing a backup program that needed to scan many files you might want to not update atime for performance reasons, even if the user otherwise has atime enabled for the given filesystem.

Without the O_NOATIME flag this would not be possible, and one would have to hope that the user mounts the filesystem with noatime which is not always going to happen.

In any event, it's already possible to pass the O_NOATIME flag as @bnoordhuis suggested, it's just not a pre-defined constant.

@ronkorving I think it's good for userland to have as much control as possible, so it's great that one can still pass integer flags at present.

@ronkorving
Copy link
Contributor

@jorangreef I agree! It would be great if we could make that more accessible to people.

Trott added a commit to Trott/io.js that referenced this issue Apr 30, 2016
Add O_NOATIME flag on Linux for use with `fs.open()`.

PR-URL: nodejs#6492
Fixes: nodejs#2182
@Trott
Copy link
Member

Trott commented Apr 30, 2016

Proposed fix: #6492

@Trott Trott closed this as completed in 52f85be May 4, 2016
evanlucas pushed a commit that referenced this issue May 17, 2016
Add O_NOATIME flag on Linux for use with `fs.open()`.

PR-URL: #6492
Fixes: #2182
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request Issues that request new features to be added to Node.js. fs Issues and PRs related to the fs subsystem / file system.
Projects
None yet
Development

No branches or pull requests

7 participants