Require perf optimization #21300

ghost · 2018-06-13T02:27:29Z

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

Aim to solve:

Optimize the performance of require one file in a module multiple times

How

Cache a map of the requests and filenames in the parent module

Example

"use strict";

console.time(1);

for(var i = 0; i < 100000; i++){
	require('./require.cpu.empty.js');
}

console.timeEnd(1);

var Module = module.constructor;
var _resolveFilename = Module._resolveFilename

Module._resolveFilename = function(request, parent, isMain, options) {
  var res;
  
  // performance optimize here
  if(parent.resolveFilenameCache){
    if(parent.resolveFilenameCache[request]){
      return parent.resolveFilenameCache[request]
    }
  }else{
  	parent.resolveFilenameCache = {};
  }
  
  res = _resolveFilename(request, parent, isMain, options);
  
  parent.resolveFilenameCache[request] = res;
  
  return res;
}

console.time('2');

for(var i = 0; i < 100000; i++){
	require('./require.cpu.empty.js');
}

console.timeEnd('2');

// result in node V8
1: 1178.783ms
2: 10.344ms

Profile

before:

after:

Let me know and I'll append some tests or benchmarks if you think it's worth a try.

do not use cache in case: require.resolve('your/request', {paths: []})

apapirovski · 2018-06-13T04:18:55Z

Hi @tswjs, could you explain the use case that this solves that caching it in a local variable doesn't? Thanks.

ghost · 2018-06-13T07:31:52Z

Hi @apapirovski , thanks for your reply.
The modules may be different with a change of requests, which means that the developer must maintain a cache map in the code.

const cache = {}

module.exports = (req, res) => {
  for(let i = 0; i < 100000; i++){
    switch (req.url) {
      case 'a':
        cache.a = cache.a ? cache.a : require('a');
        break;
      case 'b':
       cache.b = cache.b ? cache.b : require('b');
       break;
      ...
    }
  }
}

It can solve the problem, but it's really messy code. If the inside cache can solve the problem, people can use require under any circumstances without any concerns. It's a worthy change.

bnoordhuis · 2018-06-13T07:50:11Z

There is probably a better way to accomplish this than by adding a new property to the public module.Module object.

If you grep that file for stat.cache, you can find an optimization that we use for caching fs.stat results. You should be able to repurpose that.

It is now easily accessible from the Environment Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Ali Ijaz Sheikh <[email protected]>

The Deprecation cycle explanation in the COLLABORATOR_GUIDE.md file is now more concise. PR-URL: #21303 Reviewed-By: Trivikram Kamat <[email protected]> Reviewed-By: Sakthipriyan Vairamani <[email protected]> Reviewed-By: Tobias Nießen <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Minwoo Jung <[email protected]> Reviewed-By: Anatoli Papirovski <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: James M Snell <[email protected]>

Pending OpenSSL 1.1.0i release. PR-URL: #21282 Reviewed-By: Shigeki Ohtsu <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Upstream: openssl/openssl@ea7abee Original commit message: Reject excessively large primes in DH key generation. CVE-2018-0732 Signed-off-by: Guido Vranken <[email protected]> (cherry picked from commit 91f7361f47b082ae61ffe1a7b17bb2adf213c7fe) Reviewed-by: Tim Hudson <[email protected]> Reviewed-by: Matt Caswell <[email protected]> (Merged from openssl/openssl#6457)

PR-URL: #21265 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Lance Ball <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Trivikram Kamat <[email protected]> Reviewed-By: James M Snell <[email protected]>

The old implementation silently failed in EVP_CipherInit_ex in EVP_CIPH_WRAP_MODE, this commit should fix that. PR-URL: #21287 Reviewed-By: Ujjwal Sharma <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: James M Snell <[email protected]>

PR-URL: #21247 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Tiancheng "Timothy" Gu <[email protected]>

While `perf_hooks` is still in experimental, remove less useful bootstrap milestones. PR-URL: #21247 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Tiancheng "Timothy" Gu <[email protected]>

Set the thread name for workers in trace events. Also, use uint64_t for thread_id_ because there's really no reason for those to be doubles PR-URL: #21246 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Ali Ijaz Sheikh <[email protected]> Reviewed-By: Gireesh Punathil <[email protected]>

This change places the links to Writable stream and Readble stream in the order these sections appear when scrolling down the screen. PR-URL: #21333 Reviewed-By: Vse Mozhet Byt <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>

This reverts commit a24b691. PR-URL: #21363 Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: Rich Trott <[email protected]>

PR-URL: #21361 Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: Jon Moss <[email protected]> Reviewed-By: Vse Mozhet Byt <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>

Linking URL text to itself is superfluous. It will display as a link in GitHub anyway. Bonus: This makes it possible to wrap the line at 80 characters. PR-URL: #21361 Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: Jon Moss <[email protected]> Reviewed-By: Vse Mozhet Byt <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>

PR-URL: #21361 Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: Jon Moss <[email protected]> Reviewed-By: Vse Mozhet Byt <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>

Makefile tasks only lint doc/**/*.md files but omit files that match doc/*.md. This change adds these previously-omitted files to the list of files to be linted. PR-URL: #21361 Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: Jon Moss <[email protected]> Reviewed-By: Vse Mozhet Byt <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>

The Buffer doc has a helpful "Class Method" label before static methods on the Buffer class. The only other class in the docs that I can find with static methods is ECDH in crypto. Add the label to the one static method on the ECDH class. PR-URL: #21357 Reviewed-By: Vse Mozhet Byt <[email protected]> Reviewed-By: Michaël Zasso <[email protected]>

PR-URL: #21311 Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Trivikram Kamat <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: James M Snell <[email protected]>

PR-URL: #21367 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Ujjwal Sharma <[email protected]>

Fix a memory leak that occurs with header names that are short and not present in the static table of default headers. PR-URL: #21336 Reviewed-By: Anatoli Papirovski <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Minwoo Jung <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Tiancheng "Timothy" Gu <[email protected]>

Currently the following compiler warning is reported: ../src/node_stat_watcher.cc:113:13: warning: unused variable 'argc' [-Wunused-variable] const int argc = args.Length(); ^ 1 warning generated. This commit removes the unused argc variable. PR-URL: #21337 Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]>

PR-URL: #21256 Reviewed-By: Gus Caplan <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Tiancheng "Timothy" Gu <[email protected]> Reviewed-By: Anatoli Papirovski <[email protected]>

PR-URL: #21273 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Jon Moss <[email protected]> Reviewed-By: Michael Dawson <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Sakthipriyan Vairamani <[email protected]> Reviewed-By: Vse Mozhet Byt <[email protected]> Reviewed-By: Gireesh Punathil <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Matheus Marchini <[email protected]> Reviewed-By: Tiancheng "Timothy" Gu <[email protected]> Reviewed-By: Trivikram Kamat <[email protected]> Reviewed-By: Сковорода Никита Андреевич <[email protected]> Reviewed-By: James M Snell <[email protected]>

Remove obsolete wiki references from BUILDING.md. PR-URL: #21369 Reviewed-By: Anatoli Papirovski <[email protected]> Reviewed-By: Ujjwal Sharma <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Weijia Wang <[email protected]> Reviewed-By: Trivikram Kamat <[email protected]>

Instead of wording the paragraph about communicating deprecations with the ecosystem as if it were directed at the ecosystem, write it as in imperative directed at the collaborator, since that is the audience for the COLLABORATOR_GUIDE. Instead of assuring the reader that a best effort will be made, instruct the reader to make the effort to communicate with the ecosystem. PR-URL: #21340 Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Vse Mozhet Byt <[email protected]> Reviewed-By: Trivikram Kamat <[email protected]>

Re-enable `quotes` rule in .eslintrc.js and fix code to abide by the rule. As a bonus, this makes the code (IMO, anyway) more readable. (It certainly isn't *less* readable, at least not IMO.) PR-URL: #21338 Reviewed-By: Gus Caplan <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Weijia Wang <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Trivikram Kamat <[email protected]>

Don't set `writable` to true when a socket connects if the socket is already in an ending state. In the existing code, afterConnect always set `writable` to true. This has been the case for a long time, but previous to commit 9b7a691, the socket would still be destroyed by `destroySoon` and emit a `'close'` event. Since that commit removed this masking behavior, we have relied on maybeDestroy to destroy the socket when the readble state is ended, and that won't happen if `writable` is set to true. If the socket has `allowHalfOpen` set to true, then `destroy` will still not be called and `'close'` will not be emitted. PR-URL: #21290 Fixes: #21268 Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Matteo Collina <[email protected]>

BridgeAR · 2018-06-18T11:19:53Z

Ping @tswjs

PR-URL: #21354 Reviewed-By: Bradley Farias <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Guy Bedford <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>

The STYLE_GUIDE indicates that personal pronouns like "you" should be avoided in reference documentation. Remove "you" from N-API doc. PR-URL: #21382 Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Vse Mozhet Byt <[email protected]> Reviewed-By: Michael Dawson <[email protected]>

Fixes: #18254 PR-URL: #21393 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Trivikram Kamat <[email protected]> Reviewed-By: Gireesh Punathil <[email protected]> Reviewed-By: Yuta Hiroto <[email protected]>

ghost · 2018-06-19T07:08:50Z

Hi @bnoordhuis , do you mean use the new Map() to store the cache?

bnoordhuis · 2018-06-19T09:04:32Z

Something along those lines; be creative. The important point is that it stays internal.

ghost · 2018-06-19T09:18:24Z

Got it, I'll modify the code as you say

do not use cache in case: require.resolve('your/request', {paths: []})

ghost · 2018-06-19T13:44:37Z

repurpose: #21404

By the way, do you know how to remove these irrelevant commit history. I tried
git reset --hard where_i_was
git push origin -f

But they are still there. :(

mapleeit added 3 commits June 13, 2018 10:07

module: optimize require performance

ca6b414

module: fix bug when parent is null

21b14d2

module: fix require.resolve case

9e5c554

do not use cache in case: require.resolve('your/request', {paths: []})

Eugene Ostroukhov and others added 24 commits June 14, 2018 16:29

inspector: stop dragging platform pointer

8941fb7

It is now easily accessible from the Environment Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Ali Ijaz Sheikh <[email protected]>

perf_hooks: set bootstrap complete in only one place

17e378b

PR-URL: #21247 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Tiancheng "Timothy" Gu <[email protected]>

perf_hooks: remove less useful bootstrap marks

b55f6a0

While `perf_hooks` is still in experimental, remove less useful bootstrap milestones. PR-URL: #21247 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Tiancheng "Timothy" Gu <[email protected]>

Revert "workers,trace_events: set thread name for workers"

b505d2a

This reverts commit a24b691. PR-URL: #21363 Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: Rich Trott <[email protected]>

doc: wrap releases.md at 80 chars

0aa242e

PR-URL: #21361 Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: Jon Moss <[email protected]> Reviewed-By: Vse Mozhet Byt <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>

doc: wrap style guide at 80 characters

dabcd47

PR-URL: #21361 Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: Jon Moss <[email protected]> Reviewed-By: Vse Mozhet Byt <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>

doc: fix heading of optional console method args

f56c965

PR-URL: #21311 Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Trivikram Kamat <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: James M Snell <[email protected]>

doc: fix indentation in console.md

1da8855

PR-URL: #21367 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Ujjwal Sharma <[email protected]>

Trott and others added 3 commits June 17, 2018 21:33

devsnek and others added 3 commits June 18, 2018 09:41

test: move test-benchmark-path to sequential

8944a4f

Fixes: #18254 PR-URL: #21393 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Trivikram Kamat <[email protected]> Reviewed-By: Gireesh Punathil <[email protected]> Reviewed-By: Yuta Hiroto <[email protected]>

mapleeit added 5 commits June 19, 2018 19:54

module: optimize require performance

7ae718c

module: fix bug when parent is null

a8b24b6

module: fix require.resolve case

b5cbf39

do not use cache in case: require.resolve('your/request', {paths: []})

module: use internal cache instead of public one

4454a01

module: resolve conficts

de6af7c

ghost closed this Jun 19, 2018

ghost deleted the require-perf-optimization branch June 19, 2018 13:11

mapleeit restored the require-perf-optimization branch June 19, 2018 13:17

mapleeit mentioned this pull request Jun 19, 2018

module: add cache of request and filename to optimize require perf #21404

Closed

4 tasks

This pull request was closed.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

Require perf optimization #21300

Require perf optimization #21300

Uh oh!

ghost commented Jun 13, 2018 •

edited by ghost

Loading

Uh oh!

apapirovski commented Jun 13, 2018

Uh oh!

ghost commented Jun 13, 2018 •

edited by ghost

Loading

Uh oh!

bnoordhuis commented Jun 13, 2018

Uh oh!

BridgeAR commented Jun 18, 2018

Uh oh!

ghost commented Jun 19, 2018

Uh oh!

bnoordhuis commented Jun 19, 2018

Uh oh!

ghost commented Jun 19, 2018

Uh oh!

ghost commented Jun 19, 2018 •

edited by ghost

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

15 participants

Uh oh!

Require perf optimization #21300

Require perf optimization #21300

Uh oh!

Conversation

ghost commented Jun 13, 2018 • edited by ghost Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Checklist

Aim to solve:

How

Example

Profile

Uh oh!

apapirovski commented Jun 13, 2018

Uh oh!

ghost commented Jun 13, 2018 • edited by ghost Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bnoordhuis commented Jun 13, 2018

Uh oh!

BridgeAR commented Jun 18, 2018

Uh oh!

ghost commented Jun 19, 2018

Uh oh!

bnoordhuis commented Jun 19, 2018

Uh oh!

ghost commented Jun 19, 2018

Uh oh!

ghost commented Jun 19, 2018 • edited by ghost Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

15 participants

ghost commented Jun 13, 2018 •

edited by ghost

Loading

ghost commented Jun 13, 2018 •

edited by ghost

Loading

ghost commented Jun 19, 2018 •

edited by ghost

Loading