Skip to content

node: improve performance of process.hrtime()#4484

Merged
evanlucas merged 1 commit intonodejs:masterfrom
evanlucas:hrtime
Dec 30, 2015
Merged

node: improve performance of process.hrtime()#4484
evanlucas merged 1 commit intonodejs:masterfrom
evanlucas:hrtime

Conversation

@evanlucas
Copy link
Contributor

@evanlucas evanlucas commented Dec 30, 2015

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

evanlucas commented Dec 30, 2015

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

@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

trevnorris commented Dec 30, 2015

good catch. LGTM.

@jasnell
Copy link
Member

jasnell commented Dec 30, 2015

CI failures are unrelated. LGTM

@evanlucas
node: improve performance of process.hrtime()
78fd435 
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

evanlucas commented Dec 30, 2015

Landed in 78fd435. Thanks!

Fishrock123 pushed a commit to Fishrock123/node that referenced this pull request Jan 6, 2016
@evanlucas @Fishrock123
node: improve performance of process.hrtime()
921fb54 
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
@Fishrock123
2016-01-06, Version 5.4.0 (Stable)
af591d4 
* 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
@Fishrock123
2016-01-06, Version 5.4.0 (Stable)
5f0abd6 
* 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

MylesBorins commented Jan 13, 2016

@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

evanlucas commented Jan 14, 2016

Sure I'll try do get that done tonight.

@evanlucas
Copy link
Contributor Author

evanlucas commented Jan 14, 2016

@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

MylesBorins commented Jan 14, 2016

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

MylesBorins commented Jan 28, 2016

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
@evanlucas
node: improve performance of process.hrtime()
027d385 
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
@Fishrock123
2016-01-06, Version 5.4.0 (Stable)
3991aa8 
* 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

Reviewers

No reviews

Assignees

No one assigned

Labels

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

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@evanlucas @trevnorris @jasnell @MylesBorins