nsStyleContext::CalcDifference shouldn't create style structs

RESOLVED FIXED in mozilla0.9.7

Status

()

Core
CSS Parsing and Computation
RESOLVED FIXED
17 years ago
17 years ago

People

(Reporter: dbaron, Assigned: David Hyatt)

Tracking

({memory-footprint, perf})

Trunk
mozilla0.9.7
x86
Linux
memory-footprint, perf
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 obsolete attachment)

(Reporter)

Description

17 years ago
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

17 years ago
Keywords: footprint, perf
(Assignee)

Updated

17 years ago
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla0.9.7
(Assignee)

Comment 1

17 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

17 years ago
Created attachment 57570 [details] [diff] [review]
Patch.  See the CalcStyleDifference function in the bug for easier reading.
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

17 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

17 years ago
Fixed.
Status: ASSIGNED → RESOLVED
Last Resolved: 17 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.