698644 - Measure event loop latency

Status: RESOLVED WONTFIX

(Core :: XPConnect, defect)

Description

[Andreas Gal :gal]

Looks to me that all we need that for these days is to keep XPConnect around, which can do with addref/release just as well.

Comment 1

[Andreas Gal :gal]

Never mind, its not just OnDispatch, its also the context pushing.

Comment 2

[Andreas Gal :gal]

In which case I can re-purpose this code nicely.

Comment 3

[Andreas Gal :gal]

We want to know when an event takes a long time to process, minus processing of content script. I will put some measurement into xpconnect On/AfterProcessNextEvent and cancel the measurement if content script runs.

Comment 4

[Andreas Gal :gal]

Attachment: patch

untested

Comment 5

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Comment on attachment 570897 [details] [diff] [review]
patch

Review of attachment 570897 [details] [diff] [review]:
-----------------------------------------------------------------

I know this isn't expecting review or anything but I wanted to point out two things before anyone tries it.

::: js/xpconnect/src/nsXPConnect.cpp
@@ +2355,5 @@
>  nsXPConnect::OnProcessNextEvent(nsIThreadInternal *aThread, bool aMayWait,
>                                  PRUint32 aRecursionDepth)
>  {
> +    EventProcessingTiming timing = { current, PR_Now() };
> +    current = &timing;

This won't work, you're taking address of stack var that you're about to paint over when this function exits.

@@ +2374,5 @@
>  
> +    nsresult rv = Pop(nsnull);
> +
> +    PRTime now = PR_Now();
> +    long ms = (now = current->start) / PR_USEC_PER_MSEC;

Should be |(now - current->start)|

Comment 6

[Andreas Gal :gal]

Yeah I have a new patch will upload when I find network.

Comment 7

[Andreas Gal :gal]

Attachment: new patch

Comment 8

[Benjamin Smedberg]

Note that these global observers only run on *XPCOM* events: the widget thread observer actually does the blocking on native-or-XPCOM events and may dispatch many (or no) native events for each iteration of the main XPCOM event loop (nsThread::ProcessNextEvent). So we really need the instrumentation at the widget layer (or to revamp our main event dispatch mechanism) instead of at the XPCOM layer.

The data here won't be especially useful, I think.

Comment 9

[BugBot [:suhaib / :marco/ :calixte]]

The bug assignee didn't login in Bugzilla in the last 7 months, so the assignee is being reset.

Comment 10

[Nika Layzell [:nika] (ooo, ni? for response)]

We have other mechanisms like the Background Hang Monitor for measuring event latency nowadays, so I don't think we're going to do anything with this at this point.