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

Clarity on buffered-until-onload (and buffers, buffers, everywhere) #78

Closed
nicjansma opened this issue May 31, 2017 · 7 comments
Closed

Comments

@nicjansma
Copy link

nicjansma commented May 31, 2017

Hi everyone!

We've recently integrated the buffered: true flag for the PerformanceObserver, but I'm confused a bit on its behavior.

From our discussions in the past, I believe the intention was that buffered: true would give you all buffered PO entries up to the point the PerformanceObserver.observe({ buffered: true}) is called. I think this is captured in the spec correctly, and it's an awesome addition for RUM.

I think we had also discussed that some of the specs might clear that PO buffer at onload. For example, with ResourceTiming were were discussing that at onload, its PO buffer would be cleared. So if you called PerformanceObserver.observe({ buffered: true}) after onload, you would not get any ResourceTimings that happened before onload.

I know we just merged in #76 and probably just haven't updated ResourceTiming to reflect this behavior, but I think that behavior should be captured either in PerformanceTimeline or ResourceTiming.

Along those lines, I have questions on that behavior. I think I understand the answer to some of these, but I'm not sure these are all explained in the specs:

  1. Does the PO buffer behavior apply to all specs, or is it up to each spec to define how long that buffer lasts. For example, do all specs clear their PO buffer at onload (like RT/ST which have PT buffer limits) or can some fill the buffer indefinitely (like UT which have no PT buffer limit)? If the former, we should describe this behavior in the PerformanceTimeline spec. If the later, we should probably add the details to each other spec, and possibly briefly describe the differences in the PerformanceTimeline spec as well. My preference is to allow each spec to specify its PO buffer clearing behavior, since they each have different PT buffer behaviors.

  2. What happens if you call PerformanceObserver.observe({ buffered: true}) after onload for a spec that clears its buffer at onload? You don't get any buffered entries, just new ones going forward, correct? We should point this out in the spec.

  3. The buffer we're talking about for PerformanceObserver is different from the PerformanceTimeline buffer right? Does any of this behavior with the PerformanceObserver buffer affect the PerformanceTimeline buffer? i.e. I want to make sure that ResourceTiming entries in the PerformanceTimeline wouldn't also get cleared at onload just because the PerformanceObserver's buffer is cleared (which would be a regression from today's behavior).

  4. What happens if someone calls .clearResourceTimings() before onload? This clears the PerformanceTimeline buffer. Does that also affect the PerformanceObserver buffer?

  5. What happens if the PerformanceTimeline buffer for ResourceTiming/ServerTiming (e.g. 150 entries) reaches capacity? Does the PerformanceObserver buffer still fill up without bounds until onload? That would be my preference.

  6. I've heard a bit of confusion on how long new specs like ServerTiming and PaintTiming will keep their entries -- notably that 1) after onload, if no PO is registered, the PT will not get any more entries, or 2) at onload, the PT buffer will get cleared. Both of those would be a challenge for RUM.

I also made this small chart for clarity on how each spec handles buffering and what I think it's behavior should be:

Spec Static Data PerformanceTimeline Expected Entries PerformanceTimeline Buffer Size PO buffer cleared at onload
NavigationTiming performance.timing yes 1 n/a no
ResourceTiming n/a yes 100+ 150 yes
UserTiming n/a yes 0-many n/a no
ServerTiming n/a yes 100+ 150 yes
PaintTiming proposed yes 2+ n/a no
LongTasks no no < 100 n/a yes?

(I'm willing to help out with a spec PR once there's clarity on the above).

@nicjansma
Copy link
Author

Actually, I think a lot of my questions hinge on 3: whether or not PO has a separate buffer from PT.

If PO doesn't have a separate buffer -- it just relies on the underlying spec's behavior, then:

  1. None of the current specs clear their buffer on onload. RT and ST have a buffer limit, while UT/NT/PT are unbound. buffered:true doesn't behave differently before or after onload.

  2. buffered:true after onload would just grab all of the entries from the PerformanceTimeline's buffer. i.e. calling it after onload for ResourceTiming would give you everything in the ResourceTiming buffer.

  3. There is only one buffer

  4. buffered:true would be affected by .clearResourceTimings()

  5. The PerformanceObserver would be limited by any constraints of the underlying spec. If ResourceTiming filled to 150 entries, observer events would fire, but buffered:true would only return the first 150 entries.

  6. Since there's no separate PO buffer, this one wouldn't matter.

Also, if this is the case, LongTasks won't work with buffered:true unless it adds PerformanceTimeline support.

I guess this is all just an elaborate message saying that I'm confused.

@cvazac
Copy link
Contributor

cvazac commented Jun 7, 2017

buffered: true essentially eliminates the race condition around asking the performance timeline for entries and then registering a PO. But, I agree that there is some weirdness there. buffered: true is different for RT and ST because performance.getEntries*() is different as well. That is, for RT data, getEntries*() returns the first 150 after the last clear (or navigation start if no clear). And for ST data, it means first 150 after navigation start, but before onload.

The problem with only buffering ST entries until onload is that any script that really cares about all ST entries has to be loaded in a way that blocks onload. If the script loads after onload it will miss ST data for any resource loaded after 'onload' but before the script.

@igrigorik
Copy link
Member

@nicjansma great feedback and questions. @cvazac I think we missed a few cases in #76...

From our discussions in the past, I believe the intention was that buffered: true would give you all buffered PO entries up to the point the PerformanceObserver.observe({ buffered: true}) is called.

Not quite. We left this flexible and upstream specs that call "queue entry" step can define the right behavior. For example, Server Timing and Long Tasks should set the add to performance entry buffer flag to true up until end of onload. We initially had this hard-coded in perf timeline, but then relaxed this requirement to allow for cases where number of entries is low and it may make sense to queue the entries regardless of state of onload.

As a general pattern: we cannot queue all entry types by default and indefinitely, as this requirement would be at odds with our goal of allowing performance timeline to scale to hundreds of events per second.

I think we had also discussed that some of the specs might clear that PO buffer at onload.

No, there is no ability to clear the global buffer. There are many different consumers, if we allow clearing, we're back to square one with race conditions.

I know we just merged in #76 and probably just haven't updated ResourceTiming to reflect this behavior, but I think that behavior should be captured either in PerformanceTimeline or ResourceTiming.

There are no updates needed to RT. RT has own global buffer and that's something we're not planning to change.

What happens if someone calls .clearResourceTimings() before onload? This clears the PerformanceTimeline buffer. Does that also affect the PerformanceObserver buffer?

As specced, yes.

What happens if the PerformanceTimeline buffer for ResourceTiming/ServerTiming (e.g. 150 entries) reaches capacity? Does the PerformanceObserver buffer still fill up without bounds until onload? That would be my preference.

This is undefined. We do need to specify a cap on how many entries the global timeline is willing to buffer, and it's behavior for when it's full...

I've heard a bit of confusion on how long new specs like ServerTiming and PaintTiming will keep their entries -- notably that 1) after onload, if no PO is registered, the PT will not get any more entries, or 2) at onload, the PT buffer will get cleared. Both of those would be a challenge for RUM.

Paint Timing can't get any new entries after onload.. by definition, those events fire before onload. In case of Server Timing, events that are queued before end of onload will be stored in perf timeline.


Stepping back, I think a lot of the above hinges on assumptions about "clearing buffers" which is not something we discussed or want.. I think. Does the above help clarify the lay of the land?

@igrigorik
Copy link
Member

@cvazac I think we have a bug in the processing model. We should move step 7 above step 4.. as that short-circuits our add to global buffer logic if PO is queued to execute.

@igrigorik
Copy link
Member

@nicjansma opened #81 -- did I miss anything from this thread? Should we close this and iterate there?

@igrigorik
Copy link
Member

Let's merge threads in #81, closing.

@nicjansma
Copy link
Author

(sorry for the delayed reply), thanks for the clarification! Yes #81 looks good.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants