Intent to Implement and Ship: Delayed PerformanceEntry Dispatch

[Timothy Dresser]

Contact emails

tdre...@chromium.org

Summary

Previously, we always dispatched PerformanceEntries to PerformanceObservers, via a simple PostTask. We propose to switch to dispatching these entries during idle times, or 100ms after they’ve been queued, whichever comes first.

The spec encourages this new behavior:

“The performance timeline task queue is a low priority queue that, if possible, should be processed by the user agent during idle periods to minimize impact of performance monitoring code.”

We propose initially not postponing first-paint and first-contentful paint entries, as these are the biggest contributors to risk #2 below. Once we’ve shipped this initial change, we’ll ship a separate change to postpone paint timing entries.

Motivation

There are two independent motivations for this change:

1. Discourage web developers from relying on these timestamps to make decisions on site behavior.

2. Improve performance by not processing PerformanceEntries during performance critical times.

Risks

Interoperability and Compatibility

There are two primary risks:

1. This change impacts the ordering of performance entries. This could break performance monitoring for sites which make invalid assumptions about event ordering. This is fairly unlikely, and the breakage most likely wouldn’t be user facing, so I believe this is a risk worth taking.

2. Postponing the dispatch of entries means that developers will miss some PerformanceEntries they previously received, due to users navigating away before the entries are dispatched. This may negatively impact their metrics collection. We have no insight into this, and will need to rely on bug reports, and reaching out to specific analytics providers to gather data on the negative impact here.

Edge: PerformanceObserver hasn’t shipped yet.

Firefox: Doesn’t delay dispatch.

Safari: Doesn’t delay dispatch.

Web developers: No known opinions.

Ergonomics

This makes PerformanceObservers somewhat less predictable, which is part of the objective. Developers shouldn’t be relying on these APIs to make user facing decisions, only for analytics purposes.

Activation

Not relevant, as this changes the implementation of an existing API.

Demo Link

This demo queues a PerformanceEntry with a busy main thread, and displays when the entry is processed. Before this change, Chrome normally displayed ~30ms, and it now displays ~120ms.

Debuggability

No additional debugging support is needed.

Will this feature be supported on all six Blink platforms (Windows, Mac, Linux, Chrome OS, Android, and Android WebView)?

Yes.

Is this feature fully tested by web-platform-tests?

All PerformanceObserver tests verify this functionality:

• Performance-timeline

• paint-timing

• user-timing

• longtask-timing

• server-timing

• navigation-timing

Link to entry on the feature dashboard

https://www.chromestatus.com/feature/4993617147985920

Requesting approval to ship?

Yes.

[Tim Dresser]

Contact emails

tdre...@gmail.com

[Rick Byers]

On Fri, Nov 24, 2017 at 2:59 PM, Timothy Dresser <tdre...@chromium.org> wrote:

Contact emails

tdre...@chromium.org

Summary

Previously, we always dispatched PerformanceEntries to PerformanceObservers, via a simple PostTask. We propose to switch to dispatching these entries during idle times, or 100ms after they’ve been queued, whichever comes first.

The spec encourages this new behavior:

“The performance timeline task queue is a low priority queue that, if possible, should be processed by the user agent during idle periods to minimize impact of performance monitoring code.”

We propose initially not postponing first-paint and first-contentful paint entries, as these are the biggest contributors to risk #2 below. Once we’ve shipped this initial change, we’ll ship a separate change to postpone paint timing entries.

Motivation

There are two independent motivations for this change:

1. Discourage web developers from relying on these timestamps to make decisions on site behavior.

2. Improve performance by not processing PerformanceEntries during performance critical times.

Risks

Interoperability and Compatibility

There are two primary risks:

1. This change impacts the ordering of performance entries. This could break performance monitoring for sites which make invalid assumptions about event ordering. This is fairly unlikely, and the breakage most likely wouldn’t be user facing, so I believe this is a risk worth taking.

2. Postponing the dispatch of entries means that developers will miss some PerformanceEntries they previously received, due to users navigating away before the entries are dispatched. This may negatively impact their metrics collection. We have no insight into this, and will need to rely on bug reports, and reaching out to specific analytics providers to gather data on the negative impact here.

If I understand, the current thinking is that a takeRecords API should be added to enable developers to mitigate this risk (eg. from a visibility change or unload handler), right? What's the tradeoff between shipping the delay in advance of adding takeRecords vs. the other way around (or at the same time)?

Edge: PerformanceObserver hasn’t shipped yet.

Firefox: Doesn’t delay dispatch.

Safari: Doesn’t delay dispatch.

There's been discussion in WebPerf WG with these other vendors about delaying being desirable, right?  Any signals on whether they would consider changing?

 

Web developers: No known opinions.

Ergonomics

This makes PerformanceObservers somewhat less predictable, which is part of the objective. Developers shouldn’t be relying on these APIs to make user facing decisions, only for analytics purposes.

Activation

Not relevant, as this changes the implementation of an existing API.

Demo Link

This demo queues a PerformanceEntry with a busy main thread, and displays when the entry is processed. Before this change, Chrome normally displayed ~30ms, and it now displays ~120ms.

Debuggability

No additional debugging support is needed.

Will this feature be supported on all six Blink platforms (Windows, Mac, Linux, Chrome OS, Android, and Android WebView)?

Yes.

Is this feature fully tested by web-platform-tests?

All PerformanceObserver tests verify this functionality:

• Performance-timeline

• paint-timing

• user-timing

• longtask-timing

• server-timing

• navigation-timing

I'm confused - these tests pass today, so they can't be verifying that entries are delayed, right?  Wouldn't it be possible to extend the tests to, for example, at least verify that a task from a new setTimeout(0) gets run before the observer callback (something that I think should fail today but pass after this change)?

Link to entry on the feature dashboard

https://www.chromestatus.com/feature/4993617147985920

Requesting approval to ship?

Yes.

--
You received this message because you are subscribed to the Google Groups "blink-dev" group.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAHTsfZAJvV8o_3e2Zz%3DC52dW5fQ3brE91hw56fEiULygLTFjrQ%40mail.gmail.com.

[PhistucK]

> Discourage web developers from relying on these timestamps to make decisions on site behavior.

I am not sure I understand this motivation. It sounds backwards...

Are you arguing that developers should not rely on a performance monitoring API for measuring performance (because the timestamp of the entry would be inaccurate? performanceEntry.startTime) and change their websites to improve it?

Or are you discussing other timestamps here (the timestamp of the event/observer that notifies the user that new entries were added, but not the timestamps of the entries themselves? I do not see anything like that in the API, but I might be misinformed)?

☆PhistucK

--
You received this message because you are subscribed to the Google Groups "blink-dev" group.

To unsubscribe from this group and stop receiving emails from it, send an email to blink-dev+unsubscribe@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAHTsfZBz%2B3OXvbM5JyCR5_B60LWjNNSQB1BYc6%3DGXS6moxoVvQ%40mail.gmail.com.

[PhistucK]

Oops, ignore this one, I am moving this to the other thread.

☆PhistucK

[PhistucK]

> Discourage web developers from relying on these timestamps to make decisions on site behavior.

I am not sure I understand this motivation. It sounds backwards...

Are you arguing that developers should not rely on a performance monitoring API for measuring performance (because the timestamp of the entry would be inaccurate? performanceEntry.startTime) and change their websites to improve it?

Or are you discussing other timestamps here (the timestamp of the event/observer that notifies the user that new entries were added, but not the timestamps of the entries themselves? I do not see anything like that in the API, but I might be misinformed)?

☆PhistucK

To unsubscribe from this group and stop receiving emails from it, send an email to blink-dev+unsubscribe@chromium.org.

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAFUtAY-yY2jW0oFMsxeV5XM7-oPWiUVipOB%3Dv4rHaTUyecvV2w%40mail.gmail.com.

[Philip Jägenstedt]

This is a bit similar to WebRTC getStats APIs that +Henrik Boström is working on. There people seems to be using the stats to make decisions, which is a very strange situation to be in, and would make changes like this risky.

Do you think that problems here are so unlikely that searching httparchive would be a waste of time, or is there some combination of strings that might quickly find problematic usage if it does exist?

That change itself makes sense to me, even though it is deliberately making things less predictable :)

--
You received this message because you are subscribed to the Google Groups "blink-dev" group.

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAHTsfZBz%2B3OXvbM5JyCR5_B60LWjNNSQB1BYc6%3DGXS6moxoVvQ%40mail.gmail.com.

[tdre...@chromium.org]

PhistucK, I'm claiming that developers should ONLY rely on performance monitoring APIs for measuring performance. We've seen some cases where developers try to identify if a connection is slow based on PerformanceEntry's, and modify site behavior on the fly. This is the behavior we'd like to discourage (though we should provide other tools for identifying when a "light" version of a site should be served).

Rick, I actually hadn't seen the takeRecords proposal, thanks for pointing it out. There are several methods of accomplishing the goal here: performance.getEntries(), or relatively soon, registering a new performance observer with the buffered flag. takeRecords is certainly nicer than those approaches.

We don't want to delay this work arbitrarily long, as the longer we leave this the more likely developers are to start depending on the current behavior. I'm following up off-thread here with others in the web perf space, and will see what folks think.

The spec recommends this delay, but we haven't had recent discussion around this with other vendors. I've kicked off a thread here to get input from other browser vendors, and will follow up here once we've received some feedback.

Regarding testing, the spec doesn't require that entries are delayed, so we shouldn't be enforcing this. I referenced those tests as they provide coverage that the API functions correctly, viewing the delay of performanceEntry dispatch as an implementation detail.

Philip, the common mode of failure here is that developer analytics starts missing data it depends on, which we really can't detect. There's a small class of bugs where developers are misusing the API and end up relying on the order of PerformanceEntry's across multiple entryTypes which we could potentially detect, but my intuition is that it's extremely rare. Most cases where this could cause failure will be flaky today.

[PhistucK]

Perhaps it is not clear to me, will (a) the timestamps themselves (which need to be accurate for analytics) change, or will (b) only the observer callback be delayed?

If you are proposing (a), it does not make sense to me...

If you are proposing (b), no real issue.

And regarding relying on performance APIs to alter site behaviors, you seem to be thinking this is evil, but I cannot really understand why. This is just one way of using analytical data (immediately rather than later).

While there could be better ways of doing this, I do not see this as an inherently bad thing.

Perhaps you have a concrete example that demonstrates the problematic nature of this scenario?

☆PhistucK

To unsubscribe from this group and stop receiving emails from it, send an email to blink-dev+unsubscribe@chromium.org.

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-dev/164fa710-eef9-4983-aca1-576016e6ee6a%40chromium.org.

[Philip Jägenstedt]

On Mon, Nov 27, 2017 at 3:20 PM <tdre...@chromium.org> wrote:

PhistucK, I'm claiming that developers should ONLY rely on performance monitoring APIs for measuring performance. We've seen some cases where developers try to identify if a connection is slow based on PerformanceEntry's, and modify site behavior on the fly. This is the behavior we'd like to discourage (though we should provide other tools for identifying when a "light" version of a site should be served).

Rick, I actually hadn't seen the takeRecords proposal, thanks for pointing it out. There are several methods of accomplishing the goal here: performance.getEntries(), or relatively soon, registering a new performance observer with the buffered flag. takeRecords is certainly nicer than those approaches.

We don't want to delay this work arbitrarily long, as the longer we leave this the more likely developers are to start depending on the current behavior. I'm following up off-thread here with others in the web perf space, and will see what folks think.

The spec recommends this delay, but we haven't had recent discussion around this with other vendors. I've kicked off a thread here to get input from other browser vendors, and will follow up here once we've received some feedback.

Regarding testing, the spec doesn't require that entries are delayed, so we shouldn't be enforcing this. I referenced those tests as they provide coverage that the API functions correctly, viewing the delay of performanceEntry dispatch as an implementation detail.

Philip, the common mode of failure here is that developer analytics starts missing data it depends on, which we really can't detect. There's a small class of bugs where developers are misusing the API and end up relying on the order of PerformanceEntry's across multiple entryTypes which we could potentially detect, but my intuition is that it's extremely rare. Most cases where this could cause failure will be flaky today.

LGTM1, we can't exhaustively look for problems in all cases, and when your hunch is that it's extremely rare and it would be hard to find, then that's reason enough to not do it.

[Yoav Weiss]

Replying on this thread, as it seems to have already accumulated critical mass of the replies...

It is not 100% clear from your intent if this applies to entries included in the timeline as well as observed entries. I assume from your reference to the task queue that it is not, but could you clarify that?

Further comments inline

On Fri, Nov 24, 2017 at 8:01 PM Tim Dresser <tdre...@google.com> wrote:

Contact emails

tdre...@gmail.com

Summary

Previously, we always dispatched PerformanceEntries to PerformanceObservers, via a simple PostTask. We propose to switch to dispatching these entries during idle times, or 100ms after they’ve been queued, whichever comes first.

The spec encourages this new behavior:

“The performance timeline task queue is a low priority queue that, if possible, should be processed by the user agent during idle periods to minimize impact of performance monitoring code.”

We propose initially not postponing first-paint and first-contentful paint entries, as these are the biggest contributors to risk #2 below. Once we’ve shipped this initial change, we’ll ship a separate change to postpone paint timing entries.

Motivation

There are two independent motivations for this change:

1. Discourage web developers from relying on these timestamps to make decisions on site behavior.

Wouldn't Web developers still be able to make decisions based on the contents of the entries, rather than on their timing?

1. Improve performance by not processing PerformanceEntries during performance critical times.

That makes sense for PerformanceObserver as the API makes it easier for the developer to handle the entries as they are queued. Since such handling can take a long time, it might be better to pick idle times for such handling.

Risks

Interoperability and Compatibility

There are two primary risks:

1. This change impacts the ordering of performance entries. This could break performance monitoring for sites which make invalid assumptions about event ordering. This is fairly unlikely, and the breakage most likely wouldn’t be user facing, so I believe this is a risk worth taking.

So, the ordering in which PerfObserver fires for different entry types?

Could you clarify what you mean here?

The ordering that I'd be most worried about is not the ordering of entry handlers but ordering of PerfObservers with onload handlers. Right now, that ordering is not specified, but may be relied on in some ways.

e.g. it's fairly probably there are scripts out there that assume that all observers have arrived by the time `window.onload` fires. The resulting breakage is likely not to be user facing, but it would still be breakage.

 

1. Postponing the dispatch of entries means that developers will miss some PerformanceEntries they previously received, due to users navigating away before the entries are dispatched. This may negatively impact their metrics collection. We have no insight into this, and will need to rely on bug reports, and reaching out to specific analytics providers to gather data on the negative impact here.

I have a feeling that the `takeRecords` approach Rick mentioned would be able to properly mitigate both these concerns, as we can advocate for its use before collecting all entries (either on `onload` or on visibility changes).

Edge: PerformanceObserver hasn’t shipped yet.

Firefox: Doesn’t delay dispatch.

Safari: Doesn’t delay dispatch.

Web developers: No known opinions.

Ergonomics

This makes PerformanceObservers somewhat less predictable, which is part of the objective. Developers shouldn’t be relying on these APIs to make user facing decisions, only for analytics purposes.

Activation

Not relevant, as this changes the implementation of an existing API.

Demo Link

This demo queues a PerformanceEntry with a busy main thread, and displays when the entry is processed. Before this change, Chrome normally displayed ~30ms, and it now displays ~120ms.

Debuggability

No additional debugging support is needed.

Will this feature be supported on all six Blink platforms (Windows, Mac, Linux, Chrome OS, Android, and Android WebView)?

Yes.

Is this feature fully tested by web-platform-tests?

All PerformanceObserver tests verify this functionality:

• Performance-timeline

• paint-timing

• user-timing

• longtask-timing

• server-timing

• navigation-timing

I share Rick's confusion regarding these tests. They are testing the current behavior, right?

Also, can we test the unpredictable variant? If so, how?

[Jeremy Roman]

On Fri, Nov 24, 2017 at 2:01 PM, 'Tim Dresser' via blink-dev <blin...@chromium.org> wrote:

Contact emails

tdre...@gmail.com

Summary

Previously, we always dispatched PerformanceEntries to PerformanceObservers, via a simple PostTask. We propose to switch to dispatching these entries during idle times, or 100ms after they’ve been queued, whichever comes first.

The spec encourages this new behavior:

“The performance timeline task queue is a low priority queue that, if possible, should be processed by the user agent during idle periods to minimize impact of performance monitoring code.”

We propose initially not postponing first-paint and first-contentful paint entries, as these are the biggest contributors to risk #2 below. Once we’ve shipped this initial change, we’ll ship a separate change to postpone paint timing entries.

This implies that the entire performance timeline task queue will be flushed when a first-paint or first-content occurs, correct? (To avoid reordering among tasks in the queue.)

 

[tdre...@chromium.org]

Thanks for clarifying, Phistuck.

We're proposing b), only the time at which the entry's are dispatched will change.

The main reason we don't want folks using performance entries to make behavior decisions because it ties our hands in a few ways. For example, we recently updated First Paint and First Contentful Paint to include compositing time. If people are using FP or FCP to make site behavior decisions, a change like this which provides significant value from an analytics perspective could break sites, or at least cause them to load more slowly, as they wait for a later signal.

Responses to Yoav inline.

On Monday, November 27, 2017 at 10:57:05 AM UTC-5, Yoav Weiss wrote:

Replying on this thread, as it seems to have already accumulated critical mass of the replies...

It is not 100% clear from your intent if this applies to entries included in the timeline as well as observed entries. I assume from your reference to the task queue that it is not, but could you clarify that?

Entries in the timeline will not be impacted.

 

Further comments inline

On Fri, Nov 24, 2017 at 8:01 PM Tim Dresser <tdre...@google.com> wrote:

Contact emails

tdre...@gmail.com

Summary

Previously, we always dispatched PerformanceEntries to PerformanceObservers, via a simple PostTask. We propose to switch to dispatching these entries during idle times, or 100ms after they’ve been queued, whichever comes first.

The spec encourages this new behavior:

“The performance timeline task queue is a low priority queue that, if possible, should be processed by the user agent during idle periods to minimize impact of performance monitoring code.”

We propose initially not postponing first-paint and first-contentful paint entries, as these are the biggest contributors to risk #2 below. Once we’ve shipped this initial change, we’ll ship a separate change to postpone paint timing entries.

Motivation

There are two independent motivations for this change:

1. Discourage web developers from relying on these timestamps to make decisions on site behavior.

Wouldn't Web developers still be able to make decisions based on the contents of the entries, rather than on their timing?

Absolutely, however, when the contents are made available later, this approach is less useful.

 

1. Improve performance by not processing PerformanceEntries during performance critical times.

That makes sense for PerformanceObserver as the API makes it easier for the developer to handle the entries as they are queued. Since such handling can take a long time, it might be better to pick idle times for such handling.

Yup, that's the idea.

 

Risks

Interoperability and Compatibility

There are two primary risks:

1. This change impacts the ordering of performance entries. This could break performance monitoring for sites which make invalid assumptions about event ordering. This is fairly unlikely, and the breakage most likely wouldn’t be user facing, so I believe this is a risk worth taking.

So, the ordering in which PerfObserver fires for different entry types?

Could you clarify what you mean here?

The ordering that I'd be most worried about is not the ordering of entry handlers but ordering of PerfObservers with onload handlers. Right now, that ordering is not specified, but may be relied on in some ways.

e.g. it's fairly probably there are scripts out there that assume that all observers have arrived by the time `window.onload` fires. The resulting breakage is likely not to be user facing, but it would still be breakage.

Do you think this kind of breakage would be detectable via an http archive run? If so, do you think such a run would be worthwhile?

Currently, entries are effectively ordered by their creation time. When entries are batched together, they're ordered by their start time.
There's a concrete example in this layout test. 

We call mark1, mark2, measure. Previously, we dispatched separate entries, in creation order, of mark1, mark2 measure.

Now that we dispatch these in the same callback, they're sorted by startTime, resulting in the order being mark1, measure, mark2.
 

 

1. Postponing the dispatch of entries means that developers will miss some PerformanceEntries they previously received, due to users navigating away before the entries are dispatched. This may negatively impact their metrics collection. We have no insight into this, and will need to rely on bug reports, and reaching out to specific analytics providers to gather data on the negative impact here.

I have a feeling that the `takeRecords` approach Rick mentioned would be able to properly mitigate both these concerns, as we can advocate for its use before collecting all entries (either on `onload` or on visibility changes).

I agree that takeRecords enables developers to avoid this risk. This will still impact sites which haven't updated however, and it's unclear to me at this point if we should block this change on shipping takeRecords. 

Edge: PerformanceObserver hasn’t shipped yet.

Firefox: Doesn’t delay dispatch.

Safari: Doesn’t delay dispatch.

Web developers: No known opinions.

Ergonomics

This makes PerformanceObservers somewhat less predictable, which is part of the objective. Developers shouldn’t be relying on these APIs to make user facing decisions, only for analytics purposes.

Activation

Not relevant, as this changes the implementation of an existing API.

Demo Link

This demo queues a PerformanceEntry with a busy main thread, and displays when the entry is processed. Before this change, Chrome normally displayed ~30ms, and it now displays ~120ms.

Debuggability

No additional debugging support is needed.

Will this feature be supported on all six Blink platforms (Windows, Mac, Linux, Chrome OS, Android, and Android WebView)?

Yes.

Is this feature fully tested by web-platform-tests?

All PerformanceObserver tests verify this functionality:

• Performance-timeline

• paint-timing

• user-timing

• longtask-timing

• server-timing

• navigation-timing

I share Rick's confusion regarding these tests. They are testing the current behavior, right?

Also, can we test the unpredictable variant? If so, how?

Because this isn't mandatory from a spec perspective, it's just an implementation detail. These tests cover the current behavior. Only one of the tests needs to be updated to still pass given this behavior. 
 

[tdre...@chromium.org]

Jeremy, yes, First Paint and First Contentful Paint entries will flush the queue.
The code can be seen here.

[Timothy Dresser]

Update: the current plan is to block this on implementing takeRecords.
I'll ping this thread with an update once we're happy with where takeRecords is.