Believe it or not, the memory these references hold alive is significant.
Nulling them out saves 5-10K per process.
MozReview-Commit-ID: JONjE48yE8I
--HG--
extra : rebase_source : 49adc538070eecb9183e6e052e6e43db9c4c7a99
extra : histedit_source : 699f49fad1bfa69b8c511bf96187096e751c8606
I also tried to avoid this change but, again, given the number of times I was
repeating the same pattern of defining a static method just to forward a
callback to an instance method, decided it was probably necessary. Without an
easy way to do this, people are more likely to register observers rather than
callbacks, for which we'll wind up paying a continued memory and performance
penalty.
This patch adds a helper which creates a type-safe preference callback
function which forwards calls to an instance method on its closure object.
The implementation is somewhat complicated, mainly due to the constraint that
unregistering a callback requires passing the exact same function pointer that
was used to register it. The patch achieves this by creating the callback
function as a template, with the method pointer as a template parameter. As
long as the Register and Unregister calls happen in the same translation unit,
the same template instance is guaranteed to be used for both.
The main difficulty is that, until C++ 17, there's no way match a value as a
template parameter unless you know its complete type, or can at least compute
its complete type based on earlier template parameters. That means that we
need a macro to extract the type of the method, and construct the template
with the full set of explicit parameters.
MozReview-Commit-ID: 10N3R2SRtPc
--HG--
extra : rebase_source : 7d0a8ddeb77e01d4a6f421459514e93bc0875598
I initially tried to avoid this, but decided it was necessary given the number
of times I had to repeat the same pattern of casting a variable to void*, and
then casting it back in a part of code far distant from the original type.
This changes our preference callback registration functions to match the type
of the callback's closure argument to the actual type of the closure pointer
passed, and then casting it to the type of our generic callback function. This
ensures that the callback function always gets an argument of the type it's
actually expecting without adding any additional runtime memory or
QueryInterface overhead for tracking it.
MozReview-Commit-ID: 9tLKBe10ddP
--HG--
extra : rebase_source : 7524fa8dcd5585f5a31fdeb37d95714f1bb94922
nsComputedDOMStyle is currently one of the biggest sources of pref callback
memory overhead. It currently registers about 5KB of callbacks per process. A
lot of that has to do with it registering multiple callbacks for the same
preference. But even with that problem fixed, we can do better by registering
a single callback for all observed preferences.
This patch does that, but also adds the optimization of deduplicating the list
of observed preferences to avoid wasted cycles needlessly matching against
many identical strings.
MozReview-Commit-ID: LZNgd7cAwo2
--HG--
extra : rebase_source : b73f8a17427bd01c362050d1a7c66e3f6e62332b
Preference callbacks consume a non-trivial amount of memory, which adds up
fast given the number of callbacks we register.
Many of our consumers, however, register a large number of callbacks with the
same function and closure object, but different preference names or prefixes.
Since our callback matching is nothing more complicated than iteration over
all of our registered callbacks, there's nothing that prevents us from
combining all of these into a single callback, with an array of preferences
rather than a single string.
And since we already allocate an extra 8 bytes for each callback object, we
can add a variant tag without increasing our allocation size.
MozReview-Commit-ID: I497lWfMUp3
--HG--
extra : rebase_source : bac374d9a590c325728551d1669ebc6ef8ca3884
This check finds unused namespace alias declarations. There are currently no misc-unused-alias-decls warnings in mozilla-central!
https://clang.llvm.org/extra/clang-tidy/checks/misc-unused-alias-decls.html
MozReview-Commit-ID: LHziGESvaM5
--HG--
extra : rebase_source : f10fbb6364bc947b5fa2ca8c0b47494519856940
extra : source : 987ca732290093c4bd36690c6ebd3ed2ac0b5444
This check finds potentially swapped function arguments by looking at implicit conversions. There are currently no misc-swapped-arguments warnings in mozilla-central!
https://clang.llvm.org/extra/clang-tidy/checks/bugprone-swapped-arguments.html
MozReview-Commit-ID: 6SETUcQhQP
--HG--
extra : rebase_source : d364fc90e776807ab2540d714594d07a5a83b054
extra : source : 689980bf12e98780d0d60c851549d16a4cc8efb0
This check finds side effects from repeated macro arguments:
https://clang.llvm.org/extra/clang-tidy/checks/bugprone-macro-repeated-side-effects.html
There are currently 16 misc-macro-repeated-side-effects warnings in mozilla-central, but they are all in third-party gfx/cairo code:
gfx/cairo/cairo/src/cairo-tor-scan-converter.c:1432:10: warning: side effects in the 1st macro argument 'x' are repeated in macro expansion
gfx/cairo/libpixman/src/pixman-access.c:1011:16: warning: side effects in the 2nd macro argument 'ptr' are repeated in macro expansion
gfx/cairo/libpixman/src/pixman-access.c:1037:16: warning: side effects in the 2nd macro argument 'ptr' are repeated in macro expansion
gfx/cairo/libpixman/src/pixman-access.c:1062:16: warning: side effects in the 2nd macro argument 'ptr' are repeated in macro expansion
gfx/cairo/libpixman/src/pixman-access.c:1088:16: warning: side effects in the 2nd macro argument 'ptr' are repeated in macro expansion
gfx/cairo/libpixman/src/pixman-access.c:1107:16: warning: side effects in the 2nd macro argument 'ptr' are repeated in macro expansion
gfx/cairo/libpixman/src/pixman-access.c:1126:27: warning: side effects in the 2nd macro argument 'ptr' are repeated in macro expansion
gfx/cairo/libpixman/src/pixman-access.c:1194:21: warning: side effects in the 2nd macro argument 'ptr' are repeated in macro expansion
gfx/cairo/libpixman/src/pixman-access.c:1258:16: warning: side effects in the 2nd macro argument 'ptr' are repeated in macro expansion
gfx/cairo/libpixman/src/pixman-access.c:600:28: warning: side effects in the 2nd macro argument 'ptr' are repeated in macro expansion
gfx/cairo/libpixman/src/pixman-access.c:629:28: warning: side effects in the 2nd macro argument 'ptr' are repeated in macro expansion
gfx/cairo/libpixman/src/pixman-access.c:660:28: warning: side effects in the 2nd macro argument 'ptr' are repeated in macro expansion
gfx/cairo/libpixman/src/pixman-access.c:690:28: warning: side effects in the 2nd macro argument 'ptr' are repeated in macro expansion
gfx/cairo/libpixman/src/pixman-access.c:721:28: warning: side effects in the 2nd macro argument 'ptr' are repeated in macro expansion
gfx/cairo/libpixman/src/pixman-access.c:986:16: warning: side effects in the 2nd macro argument 'ptr' are repeated in macro expansion
gfx/cairo/libpixman/src/pixman-edge-imp.h:126:20: warning: side effects in the 2nd macro argument 'ptr' are repeated in macro expansion
MozReview-Commit-ID: CQ6iO9JO773
--HG--
extra : rebase_source : 0e7dc6fa3990ab187c597d8042a403c554152b02
extra : source : 5f47d400d53107df0e0478b369745bfd3f055702
Jon Coppeard
Benjamin Bouvier
Coroiu Cristina
Andrea Marchesini
Marco Castelluccio
Philipp Klein
Emilio Cobos Álvarez
Kris Maglione
Chris Peterson