Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Conversation

@evanlucas
Copy link
Contributor

Move argument validation out of C++ and into JS. Improves performance
by about 15-20%.

Benchmark comparisons between current master and this branch are below:

$ node benchmark/compare.js ./node ./node_before  -- misc bench-hrtime
running ./node
misc/bench-hrtime.js
running ./node_before
misc/bench-hrtime.js

misc/bench-hrtime.js n=1000000: ./node: 13037000 ./node_before: 10677000 . 22.11%


$ node benchmark/compare.js ./node ./node_before  -- misc bench-hrtime
running ./node
misc/bench-hrtime.js
running ./node_before
misc/bench-hrtime.js

misc/bench-hrtime.js n=1000000: ./node: 12954000 ./node_before: 10642000 . 21.73%


$ node benchmark/compare.js ./node ./node_before  -- misc bench-hrtime
running ./node
misc/bench-hrtime.js
running ./node_before
misc/bench-hrtime.js

misc/bench-hrtime.js n=1000000: ./node: 13322000 ./node_before: 10770000 . 23.69%

R= @trevnorris

@evanlucas
Copy link
Contributor Author

@trevnorris trevnorris added the c++ Issues and PRs that require attention from people who are familiar with C++. label Dec 30, 2015
@trevnorris
Copy link
Contributor

good catch. LGTM.

@jasnell
Copy link
Member

jasnell commented Dec 30, 2015

CI failures are unrelated. LGTM

Move argument validation out of C++ and into JS. Improves performance
by about 15-20%.

PR-URL: nodejs#4484
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@evanlucas evanlucas closed this Dec 30, 2015
@evanlucas evanlucas deleted the hrtime branch December 30, 2015 22:55
@evanlucas evanlucas merged commit 78fd435 into nodejs:master Dec 30, 2015
@evanlucas
Copy link
Contributor Author

Landed in 78fd435. Thanks!

Fishrock123 pushed a commit to Fishrock123/node that referenced this pull request Jan 6, 2016
Move argument validation out of C++ and into JS. Improves performance
by about 15-20%.

PR-URL: nodejs#4484
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Fishrock123 added a commit to Fishrock123/node that referenced this pull request Jan 6, 2016
* http:
  - A new status code was added: 451 - "Unavailable For Legal Reasons" (Max Barinov) nodejs#4377
  - Idle sockets that have been kept alive now handle errors (José F. Romaniello) nodejs#4482
* This release also includes several minor performance improvements:
  - assert: deepEqual is now speedier when comparing TypedArrays (Claudio Rodriguez) nodejs#4330
  - lib: Use arrow functions instead of bind where possible (Minwoo Jung) nodejs#3622
  - node: Improved accessor perf of process.env (Trevor Norris) nodejs#3780
  - node: Improved performance of process.hrtime() (Trevor Norris) nodejs#3780, (Evan Lucas) nodejs#4484
  - node: Improved GetActiveHandles performance (Trevor Norris) nodejs#3780
  - util: Use faster iteration in util.format() (Jackson Tian) nodejs#3964

PR-URL: nodejs#4547
Fishrock123 added a commit to Fishrock123/node that referenced this pull request Jan 11, 2016
* http:
  - A new status code was added: 451 - "Unavailable For Legal Reasons"
(Max Barinov) nodejs#4377
  - Idle sockets that have been kept alive now handle errors (José F.
Romaniello) nodejs#4482
* This release also includes several minor performance improvements:
  - assert: deepEqual is now speedier when comparing TypedArrays
(Claudio Rodriguez) nodejs#4330
  - lib: Use arrow functions instead of bind where possible (Minwoo
Jung) nodejs#3622
  - node: Improved accessor perf of process.env (Trevor Norris)
nodejs#3780
  - node: Improved performance of process.hrtime() (Trevor Norris)
nodejs#3780, (Evan Lucas)
nodejs#4484
  - node: Improved GetActiveHandles performance (Trevor Norris)
nodejs#3780
  - util: Use faster iteration in util.format() (Jackson Tian)
nodejs#3964

Refs: nodejs#4547
PR-URL: nodejs#4623
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@MylesBorins
Copy link
Contributor

@evanlucas this is not merging into v4.x-staging without conflicts. Would you be able to take a look and perhaps send a new PR against that branch?

@evanlucas
Copy link
Contributor Author

Sure I'll try do get that done tonight.

@evanlucas
Copy link
Contributor Author

@thealphanerd this one depends on 36e8a2c, which doesn't appear to have landed in v4.x-staging, so we should probably avoid backporting this one unless that one gets backported as well?

@MylesBorins
Copy link
Contributor

it looks like 36e8a2c is part of a series of six commits that were not landing cleanly. Work done by @trevnorris

Do you think that it is worth the effort to attempt to back port all of this?

@jasnell
Copy link
Member

jasnell commented Jan 14, 2016

I would say let's hold off for now.
On Jan 14, 2016 4:21 AM, "Myles Borins" notifications@github.com wrote:

it looks like 36e8a2c
36e8a2c
is part of a series of six commits that were not landing cleanly. Work done
by @trevnorris https://github.com/trevnorris

Do you think that it is worth the effort to attempt to back port all of
this?


Reply to this email directly or view it on GitHub
#4484 (comment).

@MylesBorins
Copy link
Contributor

As #3780 is now dont-land-on-v4.x I'm going to mark this one similarly

scovetta pushed a commit to scovetta/node that referenced this pull request Apr 2, 2016
Move argument validation out of C++ and into JS. Improves performance
by about 15-20%.

PR-URL: nodejs#4484
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
scovetta pushed a commit to scovetta/node that referenced this pull request Apr 2, 2016
* http:
  - A new status code was added: 451 - "Unavailable For Legal Reasons"
(Max Barinov) nodejs#4377
  - Idle sockets that have been kept alive now handle errors (José F.
Romaniello) nodejs#4482
* This release also includes several minor performance improvements:
  - assert: deepEqual is now speedier when comparing TypedArrays
(Claudio Rodriguez) nodejs#4330
  - lib: Use arrow functions instead of bind where possible (Minwoo
Jung) nodejs#3622
  - node: Improved accessor perf of process.env (Trevor Norris)
nodejs#3780
  - node: Improved performance of process.hrtime() (Trevor Norris)
nodejs#3780, (Evan Lucas)
nodejs#4484
  - node: Improved GetActiveHandles performance (Trevor Norris)
nodejs#3780
  - util: Use faster iteration in util.format() (Jackson Tian)
nodejs#3964

Refs: nodejs#4547
PR-URL: nodejs#4623
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

c++ Issues and PRs that require attention from people who are familiar with C++.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

Morty Proxy This is a proxified and sanitized view of the page, visit original site.