Preferences::RegisterCallback should use PREF_RegisterCallback instead of owns hash tables

NEW
Unassigned

Status

()

7 years ago
5 years ago

People

(Reporter: m_kato, Unassigned)

Tracking

(Depends on: 1 bug)

Trunk
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Reporter)

Description

7 years ago
This API was introduced by bug 182954.  At that time, it was implemented in nsContentUtils.

Now, it is moved to Preferenece (libpref). So I think that we can remove ValueObserver and hash tables to replace with calling PREF_* function directly.
Note that last I checked the coalescing that this API does (that is, NOT actually registering a function anew each time an observer is added) was key for good performance.  Will that still be the case with direct PREF_* usage?
(Reporter)

Comment 3

7 years ago
PREF_* is used into libpref only.

When observer callback register using Preference::RegisterCallback, That callback info is stored into ValueObserver, nsPrefBranch.mObservers and gCallbacks into prefapi.cpp.  If Preference::RegisterCallback calls PREF_RegisterCallback directly, it is stored into gCallbacks only.

Originally, Preference::RegisterCallback is the replacement of nsIPref::RegisterCallback that is removed by bug 175193 and bug 524449.  This XPCOM method was calling PREF_RegisterCallback only.
> If Preference::RegisterCallback calls PREF_RegisterCallback directly, it is
> stored into gCallbacks only.

That doesn't answer my question.  I understand where things are stored, but note that right now there is ONE entry in gCallbacks for each set of calls to Preference::RegisterCallback with the same callback info.  You're proposing changing that to storing lots of entries in gCallbacks, but gCallbacks manipulation is ridiculously slow because its design is crappy (it does linear searches with string compares on every item, for example).  In fact, as far as I can tell from reading PREF_UnregisterCallback making this change would in fact regress bug 561076.  And these exact issues were clearly described in the bug 561076 comments...
To be clear, we need to either redesign how gCallbacks works before fixing this bug or wontfix this bug.  In my opinion, of course.
You need to log in before you can comment on or make changes to this bug.