Closed
Bug 29474
Opened 25 years ago
Closed 18 years ago
Investigate remove nested eventQs from xpcom/proxy to improve preformance.
Categories
(Core :: XPCOM, defect, P3)
Tracking
()
RESOLVED
WONTFIX
People
(Reporter: dougt, Unassigned)
References
Details
(Keywords: perf, Whiteboard: [PDT+] w/b minus on 03/03)
Attachments
(1 file)
11.81 KB,
patch
|
Details | Diff | Splinter Review |
No description provided.
Reporter | ||
Comment 1•25 years ago
|
||
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.
(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?
Comment 3•25 years ago
|
||
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
Reporter | ||
Comment 4•25 years ago
|
||
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: 25 years ago
Resolution: --- → WONTFIX
Comment 5•25 years ago
|
||
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...
Reporter | ||
Updated•25 years ago
|
Status: RESOLVED → REOPENED
Resolution: WONTFIX → ---
Reporter | ||
Comment 6•25 years ago
|
||
getting some data on window 98.
Comment 7•25 years ago
|
||
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.
Keywords: perf
Whiteboard: pref → [PDT-]
Reporter | ||
Comment 9•25 years ago
|
||
This is a big win. We save 3.14 ms *per* proxy function call on my p3/500.
Whiteboard: [PDT-]
Comment 10•25 years ago
|
||
cha-ching! what platform? NT?
Reporter | ||
Comment 11•25 years ago
|
||
yeah, nt. 98/95 has the same problem and should have a speed improvement, but I do not have data on that.
Reporter | ||
Comment 12•25 years ago
|
||
Reporter | ||
Comment 13•25 years ago
|
||
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.
Reporter | ||
Comment 14•25 years ago
|
||
I sent out diff. I also sent a build to leger to get some performance data on it.
Comment 15•25 years ago
|
||
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
Reporter | ||
Comment 16•25 years ago
|
||
QA should focus on IMAP, FTP, and cookies. Fix is checked in.
Status: REOPENED → RESOLVED
Closed: 25 years ago → 25 years ago
Resolution: --- → FIXED
Comment 17•25 years ago
|
||
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
Comment 18•25 years ago
|
||
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
Comment 19•25 years ago
|
||
We've been using IMAP since 3/1 and seems fine. mscott - ok to mark verified?
Comment 21•19 years ago
|
||
I need to resurrect nested queues for the zap code [1], 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. [1] http://www.croczilla.com/zap
Status: VERIFIED → REOPENED
Resolution: FIXED → ---
Comment 22•19 years ago
|
||
Bug 16773 now contains a patch that (optionally) reenables nested event queues for proxies.
Depends on: 16773
Updated•18 years ago
|
Assignee: dougt → nobody
Status: REOPENED → NEW
QA Contact: lchiang → xpcom
Comment 23•18 years ago
|
||
Obsoleted by Darin's thread code rewrite earlier this year.
Status: NEW → RESOLVED
Closed: 25 years ago → 18 years ago
Resolution: --- → FIXED
Updated•13 years ago
|
Resolution: FIXED → WONTFIX
You need to log in
before you can comment on or make changes to this bug.
Description
•