Closed Bug 78961 Opened 23 years ago Closed 23 years ago

MEM: More memory footprint reduction in the style system

Categories

(Core :: CSS Parsing and Computation, defect)

defect
Not set
normal

Tracking

()

VERIFIED WONTFIX
Future

People

(Reporter: pierre, Assigned: pierre)

References

()

Details

(Keywords: embed, perf)

Bug 43457 is becoming too long. I'm moving the discussion here and on the n.p.m.style newsgroup: news://news.mozilla.org/3AF314D7.4C896990%40netscape.com
Status: NEW → ASSIGNED
Keywords: perf
Target Milestone: --- → Future
Depends on: 43457
Imported dependencies from bug 43457.
Blocks: 47159, 71874, 72381
Depends on: 39618
Keywords: embed
Excerpt from the first message in the newsgroup... ---- We are now in the third step of the memory footprint reduction work. The first step was StyleContext sharing implemented by Marc last summer, enabled last winter, and described in bug 39618. The second step was just completed (er, mostly completed) when I checked in the StyleData sharing yesterday. It is described in bug 43457. http://bugzilla.mozilla.org/show_bug.cgi?id=39618 http://bugzilla.mozilla.org/show_bug.cgi?id=43457 The third step is going in at least 3 directions: 1) Performance optimization in order to re-enable StyleContext sharing at the same time as StyleData sharing. This will get done when I'm back in July. 2) David Hyatt's "Style Sharing on Steroids" proposal. Let's call it the "style rules lexicographic tree". The work is in progress and fairly well advanced. See bug 43457 starting at "David Hyatt [2001-05-02 13:56]". 3) My own "Style Sharing on Steroids (Take 2)" proposal. Let's call it the "StyleData library". I will work on it later this summer if the results from the lexicographic tree are not satisfactory, or if there is still some room left for improvement. ---- The current bug covers all three points for now. We may decide to fork later.
As a first contribution, I can say that if someone wants to look at #1 while I am away, it would certainly the biggest bang for the buck, considering that both codes (StyleContext sharing and StyleData sharing) have been extensively tested over the past several weeks or months. A relatively easy way to optimize things would be to implement memory pools for the StyleBlobs and reduce the number of calls to GetStyleData / ReadMutableStyleData when possible (see bug 77939 for instance). Waterson: you were thinking about doing some profiling. If you get some data, please post it here. Below are some statistics that I gathered before doing the StyleData sharing about the number of calls to GetStyleData / GetMutableStyleData for each of the style structures. These stats are now outdated (I already reduced some of these calls a little bit; the XUL blob was added in the meanwhile). If someone wants to resuscitate the code, it is in nsStyleContextDebug. The output is triggered through a function that is implemented for the Mac only but very easily portable to other platforms. ------------------------------------------------------------ **** Statistics for Yahoo **** ------------------------------------------------------------ ----GetStyleData------------------------------- calls calls% eStyleStruct_Font 5129 6 eStyleStruct_Color 13078 16 eStyleStruct_List 79 - eStyleStruct_Position 9780 12 eStyleStruct_Text 8972 11 eStyleStruct_Display 22643 29 eStyleStruct_Table 1752 2 eStyleStruct_Content 101 - eStyleStruct_UserInterface 3861 5 eStyleStruct_Print 912 1 eStyleStruct_Margin 2703 3 eStyleStruct_Padding 3050 3 eStyleStruct_Border 3626 4 eStyleStruct_Outline 1397 1 TOTAL 77083 100 389 ms ----GetMutableStyleData------------------------- calls calls% eStyleStruct_Font 1937 28 eStyleStruct_Color 354 5 eStyleStruct_List 37 - eStyleStruct_Position 166 2 eStyleStruct_Text 207 3 eStyleStruct_Display 1030 15 eStyleStruct_Table 132 1 eStyleStruct_Content 4 - eStyleStruct_UserInterface 268 3 eStyleStruct_Print 961 14 eStyleStruct_Margin 339 4 eStyleStruct_Padding 519 7 eStyleStruct_Border 607 8 eStyleStruct_Outline 294 4 TOTAL 6855 100 36 ms ----------------------------------------------- TOTAL time = 426243 microsecs (= 426 ms) ------------------------------------------------------------ ------------------------------------------------------------ **** Statistics for Netscape **** ------------------------------------------------------------ ----GetStyleData------------------------------- calls calls% eStyleStruct_Font 9395 6 eStyleStruct_Color 20839 14 eStyleStruct_List 70 - eStyleStruct_Position 18112 12 eStyleStruct_Text 17602 12 eStyleStruct_Display 38613 27 eStyleStruct_Table 3975 2 eStyleStruct_Content 249 - eStyleStruct_UserInterface 6109 4 eStyleStruct_Print 1321 - eStyleStruct_Margin 5808 4 eStyleStruct_Padding 6680 4 eStyleStruct_Border 8321 5 eStyleStruct_Outline 2666 1 TOTAL 139760 100 698 ms ----GetMutableStyleData------------------------- calls calls% eStyleStruct_Font 2844 25 eStyleStruct_Color 783 7 eStyleStruct_List 31 - eStyleStruct_Position 247 2 eStyleStruct_Text 500 4 eStyleStruct_Display 1947 17 eStyleStruct_Table 120 1 eStyleStruct_Content 0 - eStyleStruct_UserInterface 339 3 eStyleStruct_Print 1359 12 eStyleStruct_Margin 515 4 eStyleStruct_Padding 881 8 eStyleStruct_Border 964 8 eStyleStruct_Outline 471 4 TOTAL 11001 100 61 ms ----------------------------------------------- TOTAL time = 762317 microsecs (= 762 ms) ------------------------------------------------------------
Another thing to do for #1 is to understand why the nsIMutableStyleContext APIs are a bit slower when they pass a structure on the stack to ReadMutableStyleData than when they receive (as they do now) a global structure returned by ReadMutableStyleData. Since the contructors for all the style blobs are basically empty, the slowdown could come from the creation of strings or other member variables inside the blobs. Marked dependency on bug 74771 if someone wants to investigate. It would be nice to switch the APIs to use member variables.
Hyatt opened a separate bug for the style rules lexicographic tree: bug 78695.
Depends on: 78695
Is this still applicable?
Sounds like #3 is still on the table if we need more improvement; to be determined later.
Actually, none of the proposals in this bug are worth pursuing. The sharing achieved by the new rule tree works very differently. There are really only two cases in the rule tree where we don't get optimal sharing. (1) The HTML4 mapped attributes should be broken up into unique rules based off type, e.g., so that <img border="0" width="10" height="10"> creates two rules, one for border and one for position, rather than one rule for border and position. The reason you want to do this is that if you had the above img tag in the same doc as the following tag: <img border="0" width="5" height="5"> you get two border structs and two position structs, since each img creates one unique rule. I view this as a minor optimization, though, since in my profiling of about 12 different HTML pages, it was really only the border struct that got needlessly cloned, and it wasn't that extensive anyway. (2) Inline style attrs with the same value could share a style rule. Then you'd get sharing on that level, e.g., if you have two tags <td style="border:3px"> and <td style="border:3px"> We could ensure that the two uses of inline style map to the same style rule. Again, this is a minor optimization, since with the rule tree you'd only fork structs that were actually specified by the inline style attribute (which typically specified only one or two properties).
In that case, we should either close this, or (to retain the minor optimizations as a possible enhancement in the future) leave it as future and set severity to minor or enhancement. Someone should also check the bugs this was marked as a blocker to. Again, great work, Dave!
Other areas for footprint reduction involve reducing the sizes of the structs themselves. In particular, the borders/margins/padding structs are way bigger than they need to be.
No longer blocks: 47159
Status: ASSIGNED → RESOLVED
Closed: 23 years ago
Resolution: --- → WONTFIX
Bug 78695 made this one obsolete.
v
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.