-
Notifications
You must be signed in to change notification settings - Fork 29.8k
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
perf_hooks: allow omitted parameters in 'performance.measure' #32651
Conversation
1facebf
to
352c6a9
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This probably needs a changes:
entry for the documentation and tests.
5270419
to
02085d2
Compare
I have no idea how to add a test for these additions, any suggestions? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This probably needs a
changes:
entry for the documentation and tests.I have no idea how to add a test for these additions, any suggestions?
Not really, because it isn’t quite obvious to me what exactly these changes end up doing.
02085d2
to
29daff0
Compare
// test.js
const { PerformanceObserver, performance } = require('perf_hooks');
const obs = new PerformanceObserver((items) => {
console.log(items.getEntries()[0].duration);
performance.clearMarks();
});
obs.observe({ entryTypes: ['measure'] });
setTimeout(() => {
performance.measure('Foo');
performance.mark('B')
performance.measure('to B', 'nodeStart', 'B');
}, 1000); ./Release/node.exe test.js
1029.4472
1033.832401 |
@himself65 I guess that example could serve as the basis for a test then? |
bf42542
to
b420270
Compare
b420270
to
821c686
Compare
0b38140
to
f85faa4
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM with tests added 👍
e9ec51a
to
25bd57d
Compare
Make `startMark` and `endMark` parameters optional.
25bd57d
to
3618a2a
Compare
Make `startMark` and `endMark` parameters optional. PR-URL: nodejs#32651 Fixes: nodejs#32647 Refs: https://www.w3.org/TR/user-timing-2/#measure-method Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Chengzhong Wu <legendecas@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
Landed in d0a3bf1 |
Make `startMark` and `endMark` parameters optional. PR-URL: #32651 Fixes: #32647 Refs: https://www.w3.org/TR/user-timing-2/#measure-method Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Chengzhong Wu <legendecas@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
Make `startMark` and `endMark` parameters optional. PR-URL: #32651 Fixes: #32647 Refs: https://www.w3.org/TR/user-timing-2/#measure-method Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Chengzhong Wu <legendecas@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
Make `startMark` and `endMark` parameters optional. PR-URL: #32651 Fixes: #32647 Refs: https://www.w3.org/TR/user-timing-2/#measure-method Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Chengzhong Wu <legendecas@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
FIxes: #32647
Refs: https://www.w3.org/TR/user-timing-2/#measure-method
Checklist
make -j4 test
(UNIX), orvcbuild test
(Windows) passes