Closed
Bug 91363
Opened 21 years ago
Closed 11 years ago
Efficiency issues with nsObserverTopic, etc
Categories
(Core :: DOM: HTML Parser, defect, P4)
Tracking
()
RESOLVED
FIXED
Future
People
(Reporter: jesup, Unassigned)
References
Details
(Keywords: perf)
Mozilla 20010717xx nsObserverTopic has a few efficiency issues: 1. GetTopic() is a linear search of topics. Perhaps it should be a hashtable of some sort. 2. Notify() takes all the keys and values (which are strings), and then adds them to nsVoidArrays along with a few extra items. This causes quite a few allocations (both of strings and nsVoidArray structures), and the Observers shouldn't be modifying the keys/values anyways. Perhaps these should be passed by reference. (The strings may not duplicate all the storage due to how our string classes work; here's the code: nsString* string = new nsString(aString); if (nsVoidArray::InsertElementAt(string, aIndex)) ...
Reporter | ||
Comment 1•21 years ago
|
||
I dealt with the allocations of VoidArray in bug 90545. The issues of allocations of strings still applies.
With fix for bug 96364 GetTopic() ( rather GetEntry() )should not be an issue since the number of calls to GetEntry() has reduced a LOT.
Lowering the priority based on my previous comment.
Priority: P2 → P4
Out of time :-( Moving to 0.9.7
Target Milestone: mozilla0.9.6 → mozilla0.9.7
Randell: Is this still an issue?
Target Milestone: mozilla0.9.7 → mozilla0.9.8
Reporter | ||
Comment 7•21 years ago
|
||
Yes, I believe so. Check dbaron's pageload jprof to get an idea if it's important.
Out of time. Mass move to 0.9.9
Target Milestone: mozilla0.9.8 → mozilla0.9.9
[was 1.1alpha]
Target Milestone: mozilla1.1alpha → Future
Updated•13 years ago
|
Assignee: harishd → nobody
Status: ASSIGNED → NEW
QA Contact: moied → parser
Reporter | ||
Comment 11•11 years ago
|
||
Case 1 is now a hashtable as suggested, and case 2 no longer does the code snippet shown; it inserts fairly directly into an nsCOMArray in nsObserverList, so this is fixed.
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•