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

manually inlining validator functions improves performance #154

Open
Uzlopak opened this issue Mar 19, 2024 · 5 comments
Open

manually inlining validator functions improves performance #154

Uzlopak opened this issue Mar 19, 2024 · 5 comments

Comments

@Uzlopak
Copy link
Contributor

Uzlopak commented Mar 19, 2024

Lets take the isAbsolute function of posix in the path module.

  /**
   * @param {string} path
   * @returns {boolean}
   */
  isAbsolute(path) {
    validateString(path, 'path');
    return path.length > 0 &&
           StringPrototypeCharCodeAt(path, 0) === CHAR_FORWARD_SLASH;
  },

Benchmark it:

./node benchmark/path/isAbsolute-posix.js 
path/isAbsolute-posix.js n=100000 path="": 26,199,553.088023424
path/isAbsolute-posix.js n=100000 path=".": 19,512,560.625525866
path/isAbsolute-posix.js n=100000 path="/foo/bar": 15,003,530.33068681
path/isAbsolute-posix.js n=100000 path="/baz/..": 20,841,496.252698973
path/isAbsolute-posix.js n=100000 path="bar/baz": 25,044,717.342815597

inline validateString

  /**
   * @param {string} path
   * @returns {boolean}
   */
  isAbsolute(path) {
    if (typeof path !== 'string')
      throw new ERR_INVALID_ARG_TYPE('path', 'string', path);
    return path.length > 0 &&
           StringPrototypeCharCodeAt(path, 0) === CHAR_FORWARD_SLASH;
  },
./node benchmark/path/isAbsolute-posix.js 
path/isAbsolute-posix.js n=100000 path="": 49,143,043.605605446
path/isAbsolute-posix.js n=100000 path=".": 36,665,695.025748484
path/isAbsolute-posix.js n=100000 path="/foo/bar": 20,950,367.322790273
path/isAbsolute-posix.js n=100000 path="/baz/..": 22,806,405.772027623
path/isAbsolute-posix.js n=100000 path="bar/baz": 41,722,470.470921524

Annoying...

@ronag
Copy link
Member

ronag commented Mar 19, 2024

can you share the benchmark?

@targos
Copy link
Member

targos commented Mar 19, 2024

This is not surprising, the inline version doesn't need hideStackFrames.
The only other solution I see is using a macro system (that was removed some years ago).

@aduh95
Copy link

aduh95 commented Mar 19, 2024

Maybe we could make the "stack hiding" feature opt-in/opt-out. I'm not sure what's the use-case behind that, but I think many folks would gladly take the extra stack lines as a tradeoff for the increase performance.

@lemire
Copy link
Member

lemire commented Mar 19, 2024

What is going here? V8 ought to be able to inline the function during the JIT compilation, no?

function validateString(value, name) {
  if (typeof value !== 'string') {
    throw new ERR_INVALID_ARG_TYPE(name, 'String', value);
  }
}

Why won't it?

@Uzlopak
Copy link
Contributor Author

Uzlopak commented Apr 8, 2024

I dont know but I think this is because you pass by val and not by ref.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants