Closed
Bug 109261
Opened 23 years ago
Closed 23 years ago
nsStyleContext::CalcDifference shouldn't create style structs
Categories
(Core :: CSS Parsing and Computation, defect)
Tracking
()
RESOLVED
FIXED
mozilla0.9.7
People
(Reporter: dbaron, Assigned: hyatt)
Details
(Keywords: memory-footprint, perf)
Attachments
(1 obsolete file)
Hyatt said he had another bug that could partly cover this, but I'm filing it
anyway.
In a profile I did of opening new mail compose windows (blank), almost a third
of the time spent within nsStyleContext::GetStyleData was calls that came from
nsStyleContext::CalcDifference.
We shouldn't be computing new structs here for two reasons:
* if nobody has asked for the struct, then nobody would care if it changed, so
we should just be able to bail without creating the struct
* we're allocating memory for structs that nobody really cares about
So this is both a performance issue and a footprint issue.
|
Reporter | |
Updated•23 years ago
|
|
Assignee | |
Updated•23 years ago
|
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla0.9.7
|
|
Assignee | |
Comment 1•23 years ago
|
||
Here is the complete text of the new function:
NS_IMETHODIMP
nsStyleContext::CalcStyleDifference(nsIStyleContext* aOther, PRInt32& aHint)
{
if (aOther) {
// If our rule nodes are the same, then we are looking at the same style
// data. We know this because CalcStyleDifference is always called on two
// style contexts that point to the same element, so we know that our position
// in the style context tree is the same and our position in the rule node tree
// is also the same.
nsRuleNode* ruleNode;
aOther->GetRuleNode(&ruleNode);
if (ruleNode == mRuleNode)
return NS_OK;
PRInt32 maxHint = NS_STYLE_HINT_MAX;
PRInt32 hint;
// We begin by examining those style structs that are capable of causing the
maximal
// difference, a FRAMECHANGE.
// FRAMECHANGE Structs: Display, XUL, Content, UserInterface
if (aHint < maxHint) {
const nsStyleDisplay* display = (const
nsStyleDisplay*)PeekStyleData(eStyleStruct_Display);
if (display) {
const nsStyleDisplay* otherDisplay = (const
nsStyleDisplay*)aOther->GetStyleData(eStyleStruct_Display);
if (display != otherDisplay) {
hint = display->CalcDifference(*otherDisplay);
if (aHint < hint)
aHint = hint;
}
}
}
#ifdef INCLUDE_XUL
if (aHint < maxHint) {
const nsStyleXUL* xul = (const nsStyleXUL*)PeekStyleData(eStyleStruct_XUL);
if (xul) {
const nsStyleXUL* otherXUL = (const
nsStyleXUL*)aOther->GetStyleData(eStyleStruct_XUL);
if (xul != otherXUL) {
hint = xul->CalcDifference(*otherXUL);
if (aHint < hint)
aHint = hint;
}
}
}
#endif
if (aHint < maxHint) {
const nsStyleContent* content = (const
nsStyleContent*)PeekStyleData(eStyleStruct_Content);
if (content) {
const nsStyleContent* otherContent = (const
nsStyleContent*)aOther->GetStyleData(eStyleStruct_Content);
if (content != otherContent) {
hint = content->CalcDifference(*otherContent);
if (aHint < hint)
aHint = hint;
}
}
}
if (aHint < maxHint) {
const nsStyleUserInterface* ui = (const
nsStyleUserInterface*)PeekStyleData(eStyleStruct_UserInterface);
if (ui) {
const nsStyleUserInterface* otherUI = (const
nsStyleUserInterface*)aOther->GetStyleData(eStyleStruct_UserInterface);
if (ui != otherUI) {
hint = ui->CalcDifference(*otherUI);
if (aHint < hint)
aHint = hint;
}
}
}
// At this point, we know that the worst kind of damage we could do is a reflow.
maxHint = NS_STYLE_HINT_REFLOW;
// The following structs cause (as their maximal difference) a reflow to occur.
// REFLOW Structs: Font, Margin, Padding, Border, List, Position, Text,
TextReset,
// Visibility, Quotes, Table, TableBorder
if (aHint < maxHint) {
const nsStyleContext* other = (const nsStyleContext*)aOther;
const nsStyleFont* font = (const
nsStyleFont*)PeekStyleData(eStyleStruct_Font);
if (font) {
const nsStyleFont* otherFont = (const
nsStyleFont*)aOther->GetStyleData(eStyleStruct_Font);
if (font != otherFont) {
hint = font->CalcDifference(*otherFont);
if (aHint < hint)
aHint = hint;
}
}
}
if (aHint < maxHint) {
const nsStyleMargin* margin = (const
nsStyleMargin*)PeekStyleData(eStyleStruct_Margin);
if (margin) {
const nsStyleMargin* otherMargin = (const
nsStyleMargin*)aOther->GetStyleData(eStyleStruct_Margin);
if (margin != otherMargin) {
hint = margin->CalcDifference(*otherMargin);
if (aHint < hint)
aHint = hint;
}
}
}
if (aHint < maxHint) {
const nsStylePadding* padding = (const
nsStylePadding*)PeekStyleData(eStyleStruct_Padding);
if (padding) {
const nsStylePadding* otherPadding = (const
nsStylePadding*)aOther->GetStyleData(eStyleStruct_Padding);
if (padding != otherPadding) {
hint = padding->CalcDifference(*otherPadding);
if (aHint < hint)
aHint = hint;
}
}
}
if (aHint < maxHint) {
const nsStyleBorder* border = (const
nsStyleBorder*)PeekStyleData(eStyleStruct_Border);
if (border) {
const nsStyleBorder* otherBorder = (const
nsStyleBorder*)aOther->GetStyleData(eStyleStruct_Border);
if (border != otherBorder) {
hint = border->CalcDifference(*otherBorder);
if (aHint < hint)
aHint = hint;
}
}
}
if (aHint < maxHint) {
const nsStyleList* list = (const
nsStyleList*)PeekStyleData(eStyleStruct_List);
if (list) {
const nsStyleList* otherList = (const
nsStyleList*)aOther->GetStyleData(eStyleStruct_List);
if (list != otherList) {
hint = list->CalcDifference(*otherList);
if (aHint < hint)
aHint = hint;
}
}
}
if (aHint < maxHint) {
const nsStylePosition* pos = (const
nsStylePosition*)PeekStyleData(eStyleStruct_Position);
if (pos) {
const nsStylePosition* otherPosition = (const
nsStylePosition*)aOther->GetStyleData(eStyleStruct_Position);
if (pos != otherPosition) {
hint = pos->CalcDifference(*otherPosition);
if (aHint < hint)
aHint = hint;
}
}
}
if (aHint < maxHint) {
const nsStyleText* text = (const
nsStyleText*)PeekStyleData(eStyleStruct_Text);
if (text) {
const nsStyleText* otherText = (const
nsStyleText*)aOther->GetStyleData(eStyleStruct_Text);
if (text != otherText) {
hint = text->CalcDifference(*otherText);
if (aHint < hint)
aHint = hint;
}
}
}
if (aHint < maxHint) {
const nsStyleTextReset* text = (const
nsStyleTextReset*)PeekStyleData(eStyleStruct_TextReset);
if (text) {
const nsStyleTextReset* otherText = (const
nsStyleTextReset*)aOther->GetStyleData(eStyleStruct_TextReset);
if (text != otherText) {
hint = text->CalcDifference(*otherText);
if (aHint < hint)
aHint = hint;
}
}
}
if (aHint < maxHint) {
const nsStyleVisibility* vis = (const
nsStyleVisibility*)PeekStyleData(eStyleStruct_Visibility);
if (vis) {
const nsStyleVisibility* otherVis = (const
nsStyleVisibility*)aOther->GetStyleData(eStyleStruct_Visibility);
if (vis != otherVis) {
hint = vis->CalcDifference(*otherVis);
if (aHint < hint)
aHint = hint;
}
}
}
if (aHint < maxHint) {
const nsStyleTable* table = (const
nsStyleTable*)PeekStyleData(eStyleStruct_Table);
if (table) {
const nsStyleTable* otherTable = (const
nsStyleTable*)aOther->GetStyleData(eStyleStruct_Table);
if (table != otherTable) {
hint = table->CalcDifference(*otherTable);
if (aHint < hint)
aHint = hint;
}
}
}
if (aHint < maxHint) {
const nsStyleTableBorder* table = (const
nsStyleTableBorder*)PeekStyleData(eStyleStruct_TableBorder);
if (table) {
const nsStyleTableBorder* otherTable = (const
nsStyleTableBorder*)aOther->GetStyleData(eStyleStruct_TableBorder);
if (table != otherTable) {
hint = table->CalcDifference(*otherTable);
if (aHint < hint)
aHint = hint;
}
}
}
if (aHint < maxHint) {
const nsStyleQuotes* quotes = (const
nsStyleQuotes*)PeekStyleData(eStyleStruct_Quotes);
if (quotes) {
const nsStyleQuotes* otherQuotes = (const
nsStyleQuotes*)aOther->GetStyleData(eStyleStruct_Quotes);
if (quotes != otherQuotes) {
hint = quotes->CalcDifference(*otherQuotes);
if (aHint < hint)
aHint = hint;
}
}
}
// At this point, we know that the worst kind of damage we could do is a
re-render
// (i.e., a VISUAL change).
maxHint = NS_STYLE_HINT_VISUAL;
// The following structs cause (as their maximal difference) a re-render to
occur.
// VISUAL Structs: Color, Background, Outline, UIReset
if (aHint < maxHint) {
const nsStyleColor* color = (const
nsStyleColor*)PeekStyleData(eStyleStruct_Color);
if (color) {
const nsStyleColor* otherColor = (const
nsStyleColor*)aOther->GetStyleData(eStyleStruct_Color);
if (color != otherColor) {
hint = color->CalcDifference(*otherColor);
if (aHint < hint)
aHint = hint;
}
}
}
if (aHint < maxHint) {
const nsStyleBackground* background = (const
nsStyleBackground*)PeekStyleData(eStyleStruct_Background);
if (background) {
const nsStyleBackground* otherBackground = (const
nsStyleBackground*)aOther->GetStyleData(eStyleStruct_Background);
if (background != otherBackground) {
hint = background->CalcDifference(*otherBackground);
if (aHint < hint)
aHint = hint;
}
}
}
if (aHint < maxHint) {
const nsStyleOutline* outline = (const
nsStyleOutline*)GetStyleData(eStyleStruct_Outline);
const nsStyleOutline* otherOutline = (const
nsStyleOutline*)aOther->GetStyleData(eStyleStruct_Outline);
if (outline != otherOutline) {
hint = outline->CalcDifference(*otherOutline);
if (aHint < hint)
aHint = hint;
}
}
if (aHint < maxHint) {
const nsStyleUIReset* ui = (const
nsStyleUIReset*)PeekStyleData(eStyleStruct_UIReset);
if (ui) {
const nsStyleUIReset* otherUI = (const
nsStyleUIReset*)aOther->GetStyleData(eStyleStruct_UIReset);
if (ui != otherUI) {
hint = ui->CalcDifference(*otherUI);
if (aHint < hint)
aHint = hint;
}
}
}
}
return NS_OK;
}
|
|
Assignee | |
Comment 2•23 years ago
|
||
Comment on attachment 57570 [details] [diff] [review]
Patch. See the CalcStyleDifference function in the bug for easier reading.
r=glazman
Attachment #57570 -
Attachment is obsolete: true
Comment on attachment 57570 [details] [diff] [review]
Patch. See the CalcStyleDifference function in the bug for easier reading.
r=glazman
Attachment #57570 -
Flags: review+
|
||
Comment 5•23 years ago
|
||
Comment on attachment 57570 [details] [diff] [review]
Patch. See the CalcStyleDifference function in the bug for easier reading.
sr=hewitt
Attachment #57570 -
Flags: superreview+
|
|
Assignee | |
Comment 6•23 years ago
|
||
Fixed.
Status: ASSIGNED → RESOLVED
Closed: 23 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•