StyleContextCache::AddContext is too expensive

RESOLVED DUPLICATE of bug 72217

Status

()

Core
CSS Parsing and Computation
RESOLVED DUPLICATE of bug 72217
18 years ago
18 years ago

People

(Reporter: Daniel Bratell, Assigned: Pierre Saslawsky)

Tracking

({perf})

Trunk
x86
Windows 2000
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(URL)

(Reporter)

Description

18 years ago
While analyzing the color table stresscase, see bug 67692, I notices that 
StyleContextCache::AddContext took a bit more time than I think is necessary. 

It stands for 7.6% of the total time for the page load distributed on:
69.6%  nsVoidArray::IndexOf(...)
18.8%  VerifyList(...)
11.0%  InsertElementAt(...) (I think that is what AppendElement expands to)
 0.5%  GetList
 0.1%  The function itself.

What I feels could be optimized is the IndexOf call that probably traverses the 
whole list before failing every time for this testcase. I assume that it is 
necessary for other cases, but we could probably do better than a linear search 
through the array. 

Also, VerifyList is always succeeding and for this special case, could be 
removed. Is it necessary in a release build?
(Reporter)

Updated

18 years ago
Keywords: perf
(Reporter)

Comment 1

18 years ago
nsVoidArray::IndexOf(aContext) being something else than -1 would cause an 
assertion. Couldn't we leave it with the assertion, and remove the call? It is a 
5% gain for this testcase.

The VerifyList call must be there though. I was tricked by the function name, 
but I know see that it's necessary.
(Reporter)

Updated

18 years ago
Blocks: 67692
(Reporter)

Comment 2

18 years ago
So, this bug was "redetected" a month later and is now fixed by attinasi as bug
72217. I'm marking this a dupe of bug 72217.

*** This bug has been marked as a duplicate of 72217 ***
Status: NEW → RESOLVED
Last Resolved: 18 years ago
Resolution: --- → DUPLICATE
You need to log in before you can comment on or make changes to this bug.