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

Consider summing all performance.measure entries for a given entry name #196

Open
kevinpschaaf opened this issue Sep 25, 2020 · 0 comments
Labels
Needs Discussion Needs Discussion

Comments

@kevinpschaaf
Copy link
Member

Currently we log an error if there is more than one performance.measure entry for a given name and pick the first one:

WARNING: Found multiple performance marks/measurements with name "update". This likely indicates an error. Picking the first one.

However there are cases where we want to accumulate time e.g. inside a loop, while excluding non-relevant loop overhead, e.g.:

for (let i=0; i<1000; i++) {
  // Don't want to include this expensive operation in the measurement
  const data = generateData();

  performance.mark('render-start');
  // Only want to measure this
  render(template(data), container);
  performance.measure('render', 'render-start');
}

In this case, it seems fine to sum all of the performance.getEntriesByName('render') durations (for entryType === 'measure' only; the other measurement types are points in time and wouldn't make sense to sum) and report that. However, this wouldn't be safe as there's currently no good signal for tachometer to know when to stop polling, since it once the first measurement(s) show up it would use it, regardless of whether there were more coming.

Maybe we could consider an optional sentinel tachometer-done mark that tachometer polls for (existence only) before collecting all the entries?

@kevinpschaaf kevinpschaaf added the Type: Enhancement New feature or request label Sep 25, 2020
@aomarks aomarks moved this to Needs Discussion in Lit Project Board Jan 24, 2022
@aomarks aomarks removed the Type: Enhancement New feature or request label Jun 24, 2022
@sorvell sorvell added the Needs Discussion Needs Discussion label Sep 8, 2022
@sorvell sorvell moved this from 💬 Needs Discussion to 📋 Triaged in Lit Project Board Sep 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Discussion Needs Discussion
Projects
None yet
Development

No branches or pull requests

3 participants