Re: Notification for new navigation timing entries

I'm thinking that passing the entry, or entries, in question seems to be 
the only definitive way of ensuring that the listener knows exactly 
which "entries" the callback is firing for.  In the multi-threaded 
environment in which the UA operates, not passing them as arguments can 
lead to awkward states for consumers.

Imagine:
1) Two independent JavaScript libraries register 
performance.addEventListener('entries', ...)
2) The UI downloads a file A and raises 'entries'
3) Callback #1 is fired first and calls 
performance.clearResourceTimings() (because it notices the RT buffer is 
full, for example)
4) Callback #2 is fired second and now is in a lost-data state where it 
knows there was a new RT entry but the RT array has already been cleared

Or an even more confusing case (and not obvious to the consumer that 
there was something wrong):
1) Two JavaScript libraries register 
performance.addEventListener('entries', ...)
2) The UI downloads a file A and B
3) File A is complete, is added to the RT array, and raises 'entries'
4) Callback #1 is fired first, gets the last RT entry, and calls 
performance.clearResourceTimings() (because it notices the RT buffer is 
full, for example)
5) File B (on a background thread) is completed while Callback #1 is 
running, is added to the RT array, and raises 'entries'
6) Callback #1 completes, callback #2 is (finally) fired for File A, and 
it queries for the last (and only) RT entry (which is actually file B)
7) Callback #1 is fired for B and gets the last entry, which is B
8) Callback #2 is fired for B and gets the last entry, which is B (again)

Contrived examples, but I think in the realm of possible depending on 
the UA's implementation and the consumer's implementation. These 
examples are all possible because we allow mutation of the various 
PerformanceTimeline arrays.

Passing the entry or entries as arguments seems the most 
straight-forward and least error-prone from a consumption POV.

As far as whether or not to fire when the buffer is full, it would help 
simplify libraries that want to consume RT for example. Today they need 
to be aware of, and maintain, the RT array.  i.e. once it hits 150, 
clearResourceTimings() or setResourceTimingBufferSize().  If the event 
is fired whether-or-not the array is full, they don't have to even think 
about array size management.

Your suggestion to have this be fired per-type seems like a good 
possible route, as consumers would likely only be interested in the 
types they know about and want to deal with.

- Nic
http://nicj.net/
@NicJ

On 10/22/2014 9:35 PM, Nat Duca wrote:
> Hopefully we copy microtasks spec for the model of when the event fires.
>
> But also, thought: Why are we passing in the events into the callback? 
> Can't the caller just do getEntries? Remember if we do that we've 
> actually got to construct the newly added events in a separate array 
> from the global buffer. This could make efficiently implementing the 
> getEntries hard: in chrome, we hold off creating the JS bound objects 
> for an entry until you call getEntries, which lets us *buffer up* the 
> performance events at high speed. Thats important for making this api 
> be performance-safe.
>
> Broadly, I think we should opt for the primitives here... the event 
> saying it was added. If you want to then get the entries, use 
> getEntries. or getEntriesOfType.
>
> Also, how does this behave when the buffer is full? The buffer sizes 
> seem to be specifed per event type, which would imply that the event 
> added is called per event type? If one buffer is full and another 
> isn't what happens? Should this event be per event type?
>
> I don't know the right solution but I smell some architectural 
> fragility here. We need to remember this is a performance api... at 
> every step of the way, it should be lean and mean. :)
>
> On Wed, Oct 22, 2014 at 4:01 PM, Ilya Grigorik <igrigorik@google.com 
> <mailto:igrigorik@google.com>> wrote:
>
>
>     On Wed, Oct 22, 2014 at 3:28 PM, Nat Duca <nduca@google.com
>     <mailto:nduca@google.com>> wrote:
>
>         Forgive me for replying late in this thread but this direction
>         is very concerning for non-navigation uses of the timeline.
>         Things like putting frames in the timeline, for instance.
>         We're loookign at huge numbers of these callbacks,
>         proportional to how much we want to put in the timeline. This
>         then means that putting things into the timeline is
>         performance disturbing!
>
>         Please, please please, consider the mutation observers model
>         where a single event is fired on the microtask unwind.
>
>
>     Hmm.. Josh's original proposal did suggest batching events, but I
>     nudged it towards individual events:
>     https://github.com/w3c/performance-timeline/issues/1#issuecomment-59538916
>
>     If we revert back to the batched model... how about:
>
>     performance.addEventListener('entries', function(events) {
>     events.forEach(function(event) {
>            // process event: event.entry.name
>     <http://event.entry.name>, event.entry.entryType, etc...
>         }
>       });
>
>     Does that look reasonable? What's the logic for batching these events?
>
>     ig
>
>