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

performance.getEntriesByType returns entries out of order #42024

Closed
nolanlawson opened this issue Feb 17, 2022 · 4 comments · Fixed by #42032
Closed

performance.getEntriesByType returns entries out of order #42024

nolanlawson opened this issue Feb 17, 2022 · 4 comments · Fixed by #42032
Labels
perf_hooks Issues and PRs related to the implementation of the Performance Timing API.

Comments

@nolanlawson
Copy link

nolanlawson commented Feb 17, 2022

Version

17.5.0

Platform

Linux 5.13.0-28-generic #31~20.04.1-Ubuntu SMP Wed Jan 19 14:08:10 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux

Subsystem

perf_hooks

What steps will reproduce the bug?

Run this file (e.g. node index.js)

// index.js
(async () => {
  const sleep = ms => new Promise(resolve => setTimeout(() => resolve(), ms))

  performance.mark('one')
  await sleep(50)
  performance.mark('two')
  await sleep(50)
  performance.mark('three')
  await sleep(50)
  performance.measure('three', 'three')
  await sleep(50)
  performance.measure('two', 'two')
  await sleep(50)
  performance.measure('one', 'one')
  const entries = performance.getEntriesByType('measure')
  console.log(entries.map(x => x.name))
})()

How often does it reproduce? Is there a required condition?

Using the above code, it consistently reproduces for me. Perhaps in certain environments you may need a longer sleep time due to jitter.

What is the expected behavior?

If you run the same snippet in Chrome, Firefox, or Safari (e.g. in a DevTools Console on example.com), it will output:

['one', 'two', 'three']

According to MDN, getEntriesByType should return entries in order of startTime:

The items will be in chronological order based on the entries' startTime.

What do you see instead?

Node v16 and v17 will output:

[ 'three', 'two', 'one' ]

Additional information

No response

@Mesteery Mesteery added the perf_hooks Issues and PRs related to the implementation of the Performance Timing API. label Feb 17, 2022
@benjamingr
Copy link
Member

cc @gioragutt is this something you can fix while improving the performance of these APIs?

@gioragutt
Copy link
Contributor

@benjamingr improve in what way? Reduce redundant code or something deeper?

@benjamingr
Copy link
Member

@gioragutt they return the wrong order of entries - with the array/fixedqueue implementation this would just be adding a test for the case above and .reverseing the result (probably?)

@meixg
Copy link
Member

meixg commented Feb 17, 2022

@gioragutt they return the wrong order of entries - with the array/fixedqueue implementation this would just be adding a test for the case above and .reverseing the result (probably?)

I guess it may need a sort by startTime.

nodejs-github-bot pushed a commit that referenced this issue Feb 20, 2022
Also order entries by startTime when calling getEntriesByType.

Fix: #42004
Fix: #42024

PR-URL: #42032
Fixes: #42004
Fixes: #42024
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
bengl pushed a commit to bengl/node that referenced this issue Feb 21, 2022
Also order entries by startTime when calling getEntriesByType.

Fix: nodejs#42004
Fix: nodejs#42024

PR-URL: nodejs#42032
Fixes: nodejs#42004
Fixes: nodejs#42024
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
bengl pushed a commit to bengl/node that referenced this issue Feb 21, 2022
Also order entries by startTime when calling getEntriesByType.

Fix: nodejs#42004
Fix: nodejs#42024

PR-URL: nodejs#42032
Fixes: nodejs#42004
Fixes: nodejs#42024
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
bengl pushed a commit that referenced this issue Feb 21, 2022
Also order entries by startTime when calling getEntriesByType.

Fix: #42004
Fix: #42024

PR-URL: #42032
Fixes: #42004
Fixes: #42024
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
bengl pushed a commit that referenced this issue Feb 21, 2022
Also order entries by startTime when calling getEntriesByType.

Fix: #42004
Fix: #42024

PR-URL: #42032
Fixes: #42004
Fixes: #42024
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
bengl pushed a commit that referenced this issue Feb 22, 2022
Also order entries by startTime when calling getEntriesByType.

Fix: #42004
Fix: #42024

PR-URL: #42032
Fixes: #42004
Fixes: #42024
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
danielleadams pushed a commit to danielleadams/node that referenced this issue Apr 21, 2022
Also order entries by startTime when calling getEntriesByType.

Fix: nodejs#42004
Fix: nodejs#42024

PR-URL: nodejs#42032
Fixes: nodejs#42004
Fixes: nodejs#42024
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
danielleadams pushed a commit that referenced this issue Apr 24, 2022
Also order entries by startTime when calling getEntriesByType.

Fix: #42004
Fix: #42024

PR-URL: #42032
Fixes: #42004
Fixes: #42024
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
danielleadams pushed a commit that referenced this issue Apr 24, 2022
Also order entries by startTime when calling getEntriesByType.

Fix: #42004
Fix: #42024

PR-URL: #42032
Fixes: #42004
Fixes: #42024
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
danielleadams pushed a commit that referenced this issue Apr 24, 2022
Also order entries by startTime when calling getEntriesByType.

Fix: #42004
Fix: #42024

PR-URL: #42032
Fixes: #42004
Fixes: #42024
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
perf_hooks Issues and PRs related to the implementation of the Performance Timing API.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants