apply memory optimizations to :viewport and :viewport-scroll

VERIFIED WONTFIX

Status

()

defect
VERIFIED WONTFIX
18 years ago
16 years ago

People

(Reporter: hyatt, Assigned: pierre)

Tracking

({css-moz, perf})

Trunk
Future
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Reporter)

Description

18 years ago
All windows have the same style data for :viewport and :viewport-scroll.  We get
back 5-10% on pretty much all XUL windows if we cache this data once, globally,
and reuse it when resolving these pseudos for any document (HTML, XML, or XUL).
(Reporter)

Updated

18 years ago
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla0.9

Comment 1

18 years ago
Does Pierre's work on bug 43457 take care of this already?
IMHO :viewport, :viewport-scroll, :canvas, :scrolled-content, :wrapped-frame,
and :placeholder-frame, and maybe some others, should all be replaced by hard-
coded frames that have hard-coded, prefilled and brutally shared style contexts.

Or something. It sounded better when Hyatt said it.

Either way, is there really any reason why it is better for us to use pseudo-
elements that are only ever mentioned once (in ua.css)? It seems to me that if
they were hard coded things would be much more sensible... But what do I know...

Updated

18 years ago
Blocks: 49141

Updated

18 years ago
Keywords: perf
Depends on: 43457
Keywords: css-moz
I would tend to say that we should fix this (i.e., move from :viewport and
friends to hardcoded frames) even if it becomes irrelevant with the fix for 
bug 43457, but at a much lower priority.
(Assignee)

Comment 4

18 years ago
If the style context for :viewport-scroll is a child of the style context for 
:viewport, then the fix for bug 43457 will mostly take care of it.  We'll 
continue to allocate 2 StyleContextImpl (average size 71 bytes) and 2 
nsStyleContextData.  The difference is that the 2 nsStyleContextData currently 
take 764*2 = 1528 bytes that will be reduced to 884+68 = 952 bytes after the fix 
is checked in.

As noted in bug 43457, the savings in XUL documents (or at least navigator.xul 
and messenger.xul) isn't as high as in HTML documents.  It seemed to me that it 
was because XUL documents were generating much more CSSDeclarations and 
StyleRules.  If the problem comes in fact from the number of style contexts that 
have a null parent (and for which we allocate the entire style data), I can look 
into it and see if the idea of a global style context makes sense.  Some 
measurements I took 2 weeks ago on HTML pages showed that less than 1% of the 
style contexts have a null parent.  Maybe the number is higher with XUL 
documents.
(Reporter)

Comment 5

18 years ago
Pierre, I'm going to give this to you.  I suspect your changes make this bug
moot, so you can just close it out as a dup.
Assignee: hyatt → pierre
Status: ASSIGNED → NEW
(Assignee)

Comment 6

18 years ago
I'm marking it Future because memory optimizations (StyleContext sharing or 
StyleData sharing) cannot be applied to the :viewport and other scrolling 
elements.  The complete list is in nsStyleContext.cpp:
        nsLayoutAtoms::viewportPseudo
        nsLayoutAtoms::canvasPseudo
        nsLayoutAtoms::viewportScrollPseudo
        nsLayoutAtoms::scrolledContentPseudo
        nsLayoutAtoms::selectScrolledContentPseudo

The problem was originally found by Marc when he worked on bug 39618.

Changed the summary line from ":viewport and :viewport-scroll style data should 
be global" to "apply memory optimizations to :viewport and :viewport-scroll".
Status: NEW → ASSIGNED
Summary: :viewport and :viewport-scroll style data should be global → apply memory optimizations to :viewport and :viewport-scroll
Target Milestone: mozilla0.9 → Future
(Assignee)

Updated

18 years ago
Depends on: 78961

Comment 7

18 years ago
Sounds XP: marking All/All.
OS: Windows 2000 → All
Hardware: PC → All
(Assignee)

Comment 8

18 years ago
bug 78695 made this one obsolete
Status: ASSIGNED → RESOLVED
Last Resolved: 18 years ago
Resolution: --- → WONTFIX
ok...
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.