Skip to content
This repository was archived by the owner on Aug 26, 2022. It is now read-only.

Conversation

@schalkneethling
Copy link

@jwhitlock This error seems to be super hard to replicate. Safest is probably to just ensure the startMark exist before measuring and beaconing. r?

@schalkneethling schalkneethling added the comp: frontend Component: Frontend label Jul 3, 2018
@schalkneethling schalkneethling requested a review from jwhitlock July 3, 2018 18:01
@jwhitlock
Copy link
Contributor

Here's the backstory on this PR. We are seeing many instances where a JavaScript exception is raised:

Uncaught SyntaxError: Failed to execute 'measure' on 'Performance': The mark 'interactive-editor-loading' does not exist.

That mark is set by the interactive examples iframe when the JS is loaded:

https://github.com/mdn/interactive-examples/blob/c977bc419f9790c4b028f21bce57cb400a314447/js/editor-libs/perf.js#L12

It seems like there is no way that the performance measure could be attempted before the mark is set. However, this is JavaScript, and there must be something else going on. The original performance mark may be rejected by the client side JS. Threading or some event loop optimization may cause the events to be processed out of order, especially if they are very close. Or, the browser might be rejecting random performance marks as a Spectre mitigation.

@jwhitlock
Copy link
Contributor

We call performance.measure through window.mdn.perf.setMeasure in two paths. One path is from setLoadEventEnd, which measures the time from markStart navigationStart to the user timing markEnd id-load-event-end. This path is not the source of the JS error, because the start mark is navigationStart, not interactive-editor-loading, like the error.

So, by process of elimination, the code change and the error are for the measures ie-time-to-complete and ie-time-to-interactive. However, when I look at GA, there are 100x as many JS errors for this code as there are successful "RUM - Interactive Examples" user timings.

This change would eliminate the JS error, but the timing would still only be 1% successful. It seems we should be debugging the error rather than throwing it out.

@schalkneethling
Copy link
Author

schalkneethling commented Jul 4, 2018

I made another change on the interactive examples side (mdn/interactive-examples#1011), and I now see a big dip on the errors. With that said, it is still early on the 4th, so I will keep an eye on it.

@schalkneethling
Copy link
Author

@jwhitlock I have been monitoring this error, and since the last change on the interactive examples side, it has gone down considerably, and seems to be going down still.

screen shot 2018-07-09 at 11 51 34

With that, I reckon it is safe to merge this, to drown out the remaining noise. Thoughts?

@jwhitlock
Copy link
Contributor

The errors are down 25%, but it still seems we have way more errors than successful user timing samples. I'd like to keep the check until we can investigate further, or see if the errors fall off as caches are updated.

Here's the numbers from GA:

Date ie-time-to-interactive Samples ie-time-to-complete Samples JS Errors Sample to Error Ratio
July 4 1840 1839 62611 34.0
July 5 1979 1978 73348 37.1
July 6 1730 1721 62908 36.4
July 7 789 789 22694 28.8
July 8 1558 1553 24034 15.5
July 9 3816 3814 52578 13.8

@jwhitlock
Copy link
Contributor

The fix in #4935 looks promising. I'd like a full day of events before calling this solved, but I think I can close the PR for this approach.

@jwhitlock jwhitlock closed this Aug 21, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

comp: frontend Component: Frontend

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants