Skip to content

http2: perf_hooks integration - #17906

Closed
jasnell wants to merge 2 commits into
nodejs:masterfrom
jasnell:http2-stats
Closed

http2: perf_hooks integration#17906
jasnell wants to merge 2 commits into
nodejs:masterfrom
jasnell:http2-stats

Conversation

@jasnell

@jasnelljasnell commented Dec 28, 2017

Copy link
Copy Markdown
Member

Collect and report basic timing information about Http2Session and Http2Stream instances.

This is just the initial impl. Additional stats to be added.

const{ PerformanceObserver }=require('perf_hooks');constobs=newPerformanceObserver((items)=>{constentry=items.getEntries()[0];console.log(entry);});obs.observe({entryTypes: ['http2']});

ping @nodejs/http2

Refs: #17746

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
Affected core subsystem(s)

http2

@jasnell
http2: perf_hooks integration
40a2d6b
Collect and report basic timing information about `Http2Session`
and `Http2Stream` instances.
@nodejs-github-botnodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs related to general changes in the lib or src directory. labels Dec 28, 2017
vsemozhetbyt
vsemozhetbyt reviewed Dec 28, 2017
View reviewed changes
Comment threaddoc/api/http2.md

## Collecting HTTP/2 Performance Metrics

The [Performance Observer][] API can be used to collect basic performance

@vsemozhetbytvsemozhetbytDec 28, 2017

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be [Performance Hooks][] or should the bottom reference be changed?

@jasnelljasnellDec 28, 2017

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

reference updated

Comment threaddoc/api/http2.md Outdated
If `name` is equal to `Http2Session`, the `PerformanceEntry` will contain the
following additional properties:

* `pingRTT` {number} The number of millseconds elapsed since the transmission

@vsemozhetbytvsemozhetbytDec 28, 2017

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

millseconds -> milliseconds

Comment threaddoc/api/http2.md Outdated
following additional properties:

* `pingRTT` {number} The number of millseconds elapsed since the transmission
of a `PING` frame and the reception of it's acknowledgement. Only present if

@vsemozhetbytvsemozhetbytDec 28, 2017

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's -> its

@jasnelljasnell mentioned this pull request Dec 28, 2017
Closed
11 tasks
@jasnell

jasnell commented Dec 28, 2017

Copy link
Copy Markdown
MemberAuthor

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

@jasnell

jasnell commented Dec 29, 2017

Copy link
Copy Markdown
MemberAuthor

CI is good. One build bot failure on windows is unrelated.

@jasnell

jasnell commented Dec 30, 2017

Copy link
Copy Markdown
MemberAuthor

Ping @nodejs/http2

@jasnelljasnell added the http2 Issues or PRs related to the http2 subsystem. label Jan 2, 2018
mcollina
mcollina reviewed Jan 2, 2018
View reviewed changes
Comment threaddoc/api/http2.md
* `timeToFirstByte` {number} The number of milliseconds elapsed between the
`PerformanceEntry` `startTime` and the reception of the first `DATA` frame.
* `timeToFirstHeader` {number} The number of milliseconds elapsed between the
`PerformanceEntry` `startTime` and the reception of the first header.

@mcollinamcollinaJan 2, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is only for data coming in, should we include also some metrics for data going out?

@jasnelljasnellJan 2, 2018

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, this is only the initial implementation. Once it lands, I plan to extend the available metrics to include additional pieces.

mcollina
mcollina approved these changes Jan 2, 2018
View reviewed changes

@mcollinamcollina left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

jasnell added a commit that referenced this pull request Jan 2, 2018
@jasnell
http2: perf_hooks integration
231b116
Collect and report basic timing information about `Http2Session`
and `Http2Stream` instances.
PR-URL: #17906
Refs: #17746
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
@jasnell

jasnell commented Jan 2, 2018

Copy link
Copy Markdown
MemberAuthor

Landed in 231b116

@jasnelljasnell closed this Jan 2, 2018
MylesBorins pushed a commit to jasnell/node that referenced this pull request Jan 9, 2018
@jasnell@MylesBorins
http2: perf_hooks integration
d968c02
Collect and report basic timing information about `Http2Session`
and `Http2Stream` instances.
PR-URL: nodejs#17906
Refs: nodejs#17746
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
MylesBorins pushed a commit that referenced this pull request Jan 9, 2018
@jasnell@MylesBorins
http2: perf_hooks integration
b19d41b
Collect and report basic timing information about `Http2Session`
and `Http2Stream` instances.
Backport-PR-URL: #18050
PR-URL: #17906
Refs: #17746
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
@MylesBorinsMylesBorins mentioned this pull request Jan 10, 2018
Merged
MylesBorins added a commit that referenced this pull request Jan 10, 2018
@MylesBorins
2019-01-10 Version 9.4.0 (Current)
074a2cb
Notable change:
* async_hooks:
- deprecate AsyncHooks Sensitive API and runInAsyncIdScope. Neither
API were documented. (Andreas Madsen)
#16972
* deps:
- update nghttp2 to 1.29.0 (James M Snell)
#17908
- upgrade npm to 5.6.0 (Kat Marchán)
#17535
- cherry-pick 50f7455 from upstream V8 (Michaël Zasso)
#16591
* events:
- remove reaches into _events internals (Anatoli Papirovski)
#17440
* http:
- add rawPacket in err of `clientError` event (XadillaX)
#17672
* http2:
- implement maxSessionMemory (James M Snell)
#17967
- add initial support for originSet (James M Snell)
#17935
- add altsvc support (James M Snell)
#17917
- perf_hooks integration (James M Snell)
#17906
* net:
- remove Socket.prototype.write (Anna Henningsen)
#17644
- remove Socket.prototype.listen (Ruben Bridgewater)
#13735
* repl:
- show lexically scoped vars in tab completion (Michaël Zasso)
#16591
* stream:
- rm {writeable/readable}State.length (Calvin Metcalf)
#12857
- add flow and buffer properties to streams (Calvin Metcalf)
#12855
* util:
- allow wildcards in NODE_DEBUG variable (Tyler)
#17609
* zlib:
- add ArrayBuffer support (Jem Bezooyen)
#16042
* Addedew collaborator**
- [starkwang](https://github.com/starkwang) Weijia Wang
* Addedew TSC member**
- [danbev](https://github.com/danbev) Daniel Bevenius
PR-URL: #18069
MylesBorins pushed a commit that referenced this pull request Jan 10, 2018
@jasnell@MylesBorins
http2: perf_hooks integration
06aaaa8
Collect and report basic timing information about `Http2Session`
and `Http2Stream` instances.
Backport-PR-URL: #18050
PR-URL: #17906
Refs: #17746
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
MylesBorins added a commit that referenced this pull request Jan 10, 2018
@MylesBorins
2019-01-10 Version 9.4.0 (Current)
7aefa40
Notable change:
* async_hooks:
- deprecate AsyncHooks Sensitive API and runInAsyncIdScope. Neither
API were documented. (Andreas Madsen)
#16972
* deps:
- update nghttp2 to 1.29.0 (James M Snell)
#17908
- upgrade npm to 5.6.0 (Kat Marchán)
#17535
- cherry-pick 50f7455 from upstream V8 (Michaël Zasso)
#16591
* events:
- remove reaches into _events internals (Anatoli Papirovski)
#17440
* http:
- add rawPacket in err of `clientError` event (XadillaX)
#17672
* http2:
- implement maxSessionMemory (James M Snell)
#17967
- add initial support for originSet (James M Snell)
#17935
- add altsvc support (James M Snell)
#17917
- perf_hooks integration (James M Snell)
#17906
- Refactoring and cleanup of Http2Session and Http2Stream destroy
(James M Snell) #17406
* net:
- remove Socket.prototype.write (Anna Henningsen)
#17644
- remove Socket.prototype.listen (Ruben Bridgewater)
#13735
* repl:
- show lexically scoped vars in tab completion (Michaël Zasso)
#16591
* stream:
- rm {writeable/readable}State.length (Calvin Metcalf)
#12857
- add flow and buffer properties to streams (Calvin Metcalf)
#12855
* util:
- allow wildcards in NODE_DEBUG variable (Tyler)
#17609
* zlib:
- add ArrayBuffer support (Jem Bezooyen)
#16042
* Addedew collaborator**
- [starkwang](https://github.com/starkwang) Weijia Wang
* Addedew TSC member**
- [danbev](https://github.com/danbev) Daniel Bevenius
PR-URL: #18069
MylesBorins added a commit that referenced this pull request Jan 10, 2018
@MylesBorins
2019-01-10 Version 9.4.0 (Current)
e688175
Notable change:
* async_hooks:
- deprecate AsyncHooks Sensitive API and runInAsyncIdScope. Neither
API were documented. (Andreas Madsen)
#16972
* deps:
- update nghttp2 to 1.29.0 (James M Snell)
#17908
- upgrade npm to 5.6.0 (Kat Marchán)
#17535
- cherry-pick 50f7455 from upstream V8 (Michaël Zasso)
#16591
* events:
- remove reaches into _events internals (Anatoli Papirovski)
#17440
* http:
- add rawPacket in err of `clientError` event (XadillaX)
#17672
* http2:
- implement maxSessionMemory (James M Snell)
#17967
- add initial support for originSet (James M Snell)
#17935
- add altsvc support (James M Snell)
#17917
- perf_hooks integration (James M Snell)
#17906
- Refactoring and cleanup of Http2Session and Http2Stream destroy
(James M Snell) #17406
* net:
- remove Socket.prototype.write (Anna Henningsen)
#17644
- remove Socket.prototype.listen (Ruben Bridgewater)
#13735
* repl:
- show lexically scoped vars in tab completion (Michaël Zasso)
#16591
* stream:
- rm {writeable/readable}State.length (Calvin Metcalf)
#12857
- add flow and buffer properties to streams (Calvin Metcalf)
#12855
* util:
- allow wildcards in NODE_DEBUG variable (Tyler)
#17609
* zlib:
- add ArrayBuffer support (Jem Bezooyen)
#16042
* Addedew collaborator**
- [starkwang](https://github.com/starkwang) Weijia Wang
* Addedew TSC member**
- [danbev](https://github.com/danbev) Daniel Bevenius
PR-URL: #18069
kjin pushed a commit to kjin/node that referenced this pull request May 1, 2018
@jasnell@kjin
http2: perf_hooks integration
e92be6a
Collect and report basic timing information about `Http2Session`
and `Http2Stream` instances.
Backport-PR-URL: nodejs#18050
PR-URL: nodejs#17906
Refs: nodejs#17746
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
MylesBorins pushed a commit that referenced this pull request May 2, 2018
@jasnell@MylesBorins
http2: perf_hooks integration
dcc7f4d
Collect and report basic timing information about `Http2Session`
and `Http2Stream` instances.
Backport-PR-URL: #18050
Backport-PR-URL: #20456
PR-URL: #17906
Refs: #17746
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
@MylesBorinsMylesBorins mentioned this pull request May 2, 2018
Merged
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mcollinamcollinamcollina approved these changes
+1 more reviewer
@vsemozhetbytvsemozhetbytvsemozhetbyt left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

c++Issues and PRs that require attention from people who are familiar with C++.http2Issues or PRs related to the http2 subsystem.lib / srcIssues and PRs related to general changes in the lib or src directory.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@jasnell@mcollina@vsemozhetbyt@MylesBorins@nodejs-github-bot