Closed Bug 29474 Opened 20 years ago Closed 14 years ago
Investigate remove nested event
Qs from xpcom/proxy to improve preformance .
No description provided.
nested eventQs create/destroy an invisible window for *every* call. We may be able to remove this sucessfully. My concern is that we would be processing events on the Q which might not be ours. Need to think a bit about this.
Severity: normal → major
Status: NEW → ASSIGNED
(spawn of bug 25979, right?) The reason we make a new window for each queue is historical: NSPR was designed with one thread, one queue in mind. We added stacked queues and brought along some extra weight. I think Rick is dead on in 25979: all the queues for a thread should be able to share the same window. However, I'm not sure it's all that important. Do we need to spend a lot of time doing this? I remember once noticing a situation where a proxy repeatedly created and destroyed queues in a loop. (I seem to remember that it was the main queue for the thread. If that's true, you're stuck with the window creation, of course.) Regardless, can you teach the proxy to recycle?
What I'm seeing is that each time a method is called on a proxy object PushThreadEventQueue/PopThreadEventQueue are called. This means that in the current world, we create/destroy a window *each* time we call a proxy method. Of course it all depends on how much we use proxy objects. Right now I believe that FTP and maybe IMAP are the major consumers of proxy objects. In order to analyze the performance impact of not fixing this, you should look at the number of proxy objects calls made when FTPing a file, or doing IMAP stuff... For normal browsing, *no* proxy objects are used (yet) -- rick
I am not seeing a big preformance hit at all. On a run of two or two thosand, the delta between nested q's and not is in the few milliseconds. I am going to mark this as wont fix.
Status: ASSIGNED → RESOLVED
Closed: 20 years ago
Resolution: --- → WONTFIX
doug, I think this could have some platform specific ramifications. Can you ellaborate a little more on you testing/verifying? This bug might explain why nt hauls ass for FTP directory navigation, but win95 doesn't...
Status: RESOLVED → REOPENED
Resolution: WONTFIX → ---
getting some data on window 98.
hey doug, I guess the fact that we are calling Push/PopThreadEventQueue() for each method call is not in itself a problem :-) I think that the *real* problem is that PushThreadEventQueue() is creating the wrong kind of event queue for worker threads. And that for UI threads, a new window is being created rather than sharing a single (per thread) window. If these issues are resolved in bug #25979, then doing a Push/PopThreadEventQueue(...) may not matter too much. -- rick
Putting on PDT- radar for beta1. If you get concrete data and fix to enhance perf by a large percentage, please remove PDT- and resubmit.
Whiteboard: pref → [PDT-]
This is a big win. We save 3.14 ms *per* proxy function call on my p3/500.
cha-ching! what platform? NT?
yeah, nt. 98/95 has the same problem and should have a speed improvement, but I do not have data on that.
when the diff is applied, simply remove the NESTED_Q define, and nested eventQs will be disabled. I have tested IMAP and FTP, and both seam to work fine.
I sent out diff. I also sent a build to leger to get some performance data on it.
Putting on PDT+ radar for beta1. But will make PDT- and release note if not fixed by 03/03. This needs LOTS of testing once fix is in!!! What do you want QA to do to test?
Whiteboard: [PDT+] w/b minus on 03/03
QA should focus on IMAP, FTP, and cookies. Fix is checked in.
Status: REOPENED → RESOLVED
Closed: 20 years ago → 20 years ago
Resolution: --- → FIXED
Assigning to tever. he works in this area, and if cookies and ftp have a good day with 03/02 and 03/03 builds, I would call this verified.
QA Contact: leger → tever
I cannot see any problems with FTP or cookies. Giving to Lisa to check out IMAP. mscott would be a possible contact if you need help testing.
QA Contact: tever → lchiang
We've been using IMAP since 3/1 and seems fine. mscott - ok to mark verified?
yup. We are good to go on this one.
Status: RESOLVED → VERIFIED
I need to resurrect nested queues for the zap code , maybe as an option by adding a PROXY_NESTED_QUEUES flag to nsIProxyObjectManager.idl I have 2 threads talking to the UI thread via proxies: a media pipeline thread and the socket thread. Without nested queues, events from these two threads are not sequenced correctly onto the UI thread. Occasionally while I'm synchronously calling a function on the media thread and waiting in nsProxyObject::PostAndWait(), the UI thread gets reentrantly called from the socket thread. Potentially I might need an even more elaborate scheme where eventQs are tagged with the thread that they are accepting events from. When a call through a proxy is made, the proxy would check if there is a younger queue accepting events from the caller's thread.  http://www.croczilla.com/zap
Status: VERIFIED → REOPENED
Resolution: FIXED → ---
Bug 16773 now contains a patch that (optionally) reenables nested event queues for proxies.
Depends on: 16773
Assignee: dougt → nobody
Status: REOPENED → NEW
QA Contact: lchiang → xpcom
Obsoleted by Darin's thread code rewrite earlier this year.
Status: NEW → RESOLVED
Closed: 20 years ago → 14 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.