91363 - Efficiency issues with nsObserverTopic, etc

Status: RESOLVED FIXED

(Core :: DOM: HTML Parser, defect, P4)

Description

[Randell Jesup [:jesup] (needinfo me)]

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))
    ...

Comment 1

[Randell Jesup [:jesup] (needinfo me)]

I dealt with the allocations of VoidArray in bug 90545.  The issues of
allocations of strings still applies.

Comment 2

[harishd]

--> 0.9.6

Comment 3

[harishd]

With fix for bug 96364 GetTopic() ( rather GetEntry() )should not be an issue
since the number of calls to GetEntry() has reduced a LOT.

Comment 4

[harishd]

Lowering the priority based on my previous comment.

Comment 5

[harishd]

Out of time :-( Moving to 0.9.7

Comment 6

[harishd]

Randell: Is this still an issue?

Comment 7

[Randell Jesup [:jesup] (needinfo me)]

Yes, I believe so.  Check dbaron's pageload jprof to get an idea if it's important.

Comment 8

[harishd]

Out of time. Mass move to 0.9.9

Comment 9

[harishd]

Mass moving to 1.1.

Comment 10

[Heikki Toivonen (remove -bugzilla when emailing directly)]

[was 1.1alpha]

Comment 11

[Randell Jesup [:jesup] (needinfo me)]

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.