Closed
Bug 133821
Opened 24 years ago
Closed 24 years ago
remove |aForceUnique| from ResolveStyleContext methods
Categories
(Core :: CSS Parsing and Computation, defect)
Core
CSS Parsing and Computation
Tracking
()
RESOLVED
FIXED
mozilla1.0
People
(Reporter: dbaron, Assigned: dbaron)
Details
(Keywords: perf)
Attachments
(1 file)
|
72.61 KB,
patch
|
attinasi
:
review+
hyatt
:
superreview+
asa
:
approval+
|
Details | Diff | Splinter Review |
We should remove the |aForceUnique| parameter from all the ResolveStyleContext
methods (3 on nsIStyleSet, 3 on nsIPresContext), since the only caller that
passes PR_TRUE for this parameter is nsFrameManager::ReResolveStyleContext.
This could give a bit of a performance win doing style data computation, but I
the biggest performance would be that it could cause much less data to be blown
away for a ReResolve, since it could mean that we Re-Resolve to all the same
style contexts. (We should be careful to allow this to work for the root as
well if it matches the same rule node, which is the only place where we'd want a
same-rule-node check, I think.)
|
Assignee | |
Comment 1•24 years ago
|
||
Actually, this won't help all that much in clearing less data right now, since
we currently rebuild the rule tree for stylesheet add/remove, and we hopefully
don't call ReResolve too often when there's nothing to ReResolve...
|
||
Comment 2•24 years ago
|
||
We only rebuild the rule tree on a stylesheet removal. A stylesheet
addition does not require a rebuild of the rule tree.
|
|
||
Comment 3•24 years ago
|
||
Stylesheet addition also gets hit reasonably often in the real world, since
<style> is often encountered in the <body> of a Web page.
|
|
Assignee | |
Comment 4•24 years ago
|
||
Oh, right. I used to think that was true and somehow I looked at the code and
convinced myself otherwise. Now that I look again, I see it is true.
|
|
Assignee | |
Comment 5•24 years ago
|
||
Also, I also just noticed that whether a style context was forced to be unique
is in its mBits, so we don't need to remove the feature to get the perf win --
ReResolveStyleContext could pass aForceUnique based on the old context's bits.
Do you think there's any reason to keep this API?
|
|
||
Comment 6•24 years ago
|
||
Nope.
|
|
Assignee | |
Comment 7•24 years ago
|
||
|
|
||
Comment 8•24 years ago
|
||
Comment on attachment 76496 [details] [diff] [review]
patch
sr=hyatt.
Attachment #76496 -
Flags: superreview+
|
|
||
Updated•24 years ago
|
Attachment #76496 -
Flags: review+
|
|
||
Comment 9•24 years ago
|
||
Comment on attachment 76496 [details] [diff] [review]
patch
NS_ADDREF(newContext = oldContext);
This looks ugly to me, I'd do the assignment seperately from the ADDREF.
+ // XXXldb |oldContext| is null by this point, so this will
+ // never do anything.
So, why not remove it (or put in an assertion or warning if you are unsure)?
r=attinasi
|
||
Comment 10•24 years ago
|
||
Comment on attachment 76496 [details] [diff] [review]
patch
a=asa (on behalf of drivers) for checkin to the 1.0 trunk
Attachment #76496 -
Flags: approval+
|
|
Assignee | |
Updated•24 years ago
|
Target Milestone: --- → mozilla1.0
|
|
Assignee | |
Comment 11•24 years ago
|
||
Fix checked in 2002-03-31 08:58/59 PST.
Status: NEW → RESOLVED
Closed: 24 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•