Summary: | NodeList access by index is slow | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Alexey Proskuryakov <ap> | ||||
Component: | DOM | Assignee: | Alexey Proskuryakov <ap> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | ||||||
Priority: | P2 | ||||||
Version: | 523.x (Safari 3) | ||||||
Hardware: | Mac | ||||||
OS: | OS X 10.4 | ||||||
URL: | http://www.hixie.ch/tests/adhoc/perf/dom/artificial/core/001.html | ||||||
Attachments: |
|
Description
Alexey Proskuryakov
2007-04-22 01:55:45 PDT
Created attachment 14161 [details]
easy way out
Just add support for iterating backwards. On my machine, results on the "Index" test look OK now (~7000 ms TOT, ~100 ms TOT + this patch, ~120 ms Opera, ~1300 ms Firefox).
KHTML also has support for iterating backwards, and according to Maks Orlovich, it was added to fix a real site, forums.tweakers.nl. I've also added support for iteration from lastChild.
I still think that there might be a better data structure for a list of child nodes in ContainerNode, and that would speed up other operations, too. Obviously, performance of data structures is always a trade-off, so it would probably take someone with access to PLT to play with it.
Comment on attachment 14161 [details]
easy way out
+ unsigned dist = (unsigned)abs((long)index - (long)m_caches->lastItemOffset);
Since the parameter to abs is an int, the cast here should be to int, not long. But also no casts are needed. If you do unsigned - unsigned, then pass to int, and then extract it into unsigned it will do the right thing with no casts at all.
Otherwise looks fine. r=me
Ah, I got confused by std::abs from cstdlib. Hoping MSVC won't be confused similarly :-). Committed revision 21090. |