https://bugzilla.wikimedia.org/show_bug.cgi?id=50836

Ori Livneh <[email protected]> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[email protected]

--- Comment #5 from Ori Livneh <[email protected]> ---
(In reply to comment #1)
> clearly, the right thing to do here is to collect all the different CSS bits
> and pieces you want to inject, and call injectCSS() exactly once.

I've only noted a single repaint in Chrome Dev Tools on
https://en.wikipedia.org/wiki/Tower_of_babel, and I'm not sure yet if it's
attributable to ULS. But any form of DOM access is expensive, so the
recommendation above is entirely sound.

> the function "load" is also asinine, and can cause a high load:
> 
> it has two problems: one of them is using $.inArray() to test whether this
> fontface was already loaded: this means linear search on the array every
> single time

Hash tables lookups are worse than linear searches for arrays of this size due
to the cost of computing the hash and the relatively poorer locality. (In fact,
modern JS engines don't even implement property lookups as hash tables, but I
figure that was what you had in mind when you recommended hasOwnProperty
instead of $.inArray).

https://en.wikipedia.org/wiki/Tower_of_babel has four fontFamilies: "Amiri",
"Akkadian", "AbyssinicaSIL", and "CharisSIL". I used this to profile object vs.
array lookup:

> var myArray = ["Amiri", "Akkadian", "AbyssinicaSIL", "CharisSIL"];
> console.time('$.inArray'); for(var i = 0; i < 10000; i++) $.inArray('abc', 
> myArray); console.timeEnd('$.inArray');
$.inArray: 18.029ms
> var myObj = {"Amiri": null, "Akkadian": null, "AbyssinicaSIL": null, 
> "CharisSIL": null};
> console.time('hasOwnProperty'); for(var i = 0; i < 10000; i++) 
> myObj.hasOwnProperty('abc'); console.timeEnd('hasOwnProperty');
hasOwnProperty: 13.161ms

The difference per call works out to just under half of one millionth of a
second, certainly not enough to warrant calling either approach 'asinine'. I am
running a recent build of Chromium, but I'd wager the difference even for older
browsers would not cross the threshold of significance.

It is a bit bizarre that load() has to be called 877 times on that page,
though. That should be investigated.

ULS contains quite a lot of new JavaScript and CSS code, so it is reasonable to
expect that a close scrutiny of the code will reveal some measure of duplicate
work and various other performance problems. It's good that you've dug into the
code and pinpointed some specific problems, but please be less adversarial.
Measuring and optimizing client-side performance is hard enough to do when you
aren't under attack; calling a developer's code "asinine" or "disgusting" is
not likely to make it any easier.

-- 
You are receiving this mail because:
You are the assignee for the bug.
You are on the CC list for the bug.
_______________________________________________
Wikibugs-l mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/wikibugs-l

Reply via email to