Closed
Bug 766952
Opened 12 years ago
Closed 12 years ago
Remove unnecessary <span>s and per element styles on hgweb pages
Categories
(Developer Services :: Mercurial: hg.mozilla.org, defect)
Developer Services
Mercurial: hg.mozilla.org
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: emorley, Assigned: emorley)
References
Details
(Whiteboard: [sheriff-want])
Attachments
(1 file)
(deleted),
patch
|
ted
:
review+
|
Details | Diff | Splinter Review |
In bug 686795 comment 9, it was noticed that on pages like:
https://hg.mozilla.org/mozilla-central/diff/10e019421e6b/content/base/src/nsXMLHttpRequest.h
Every single line is wrapped in a <span> even if it doesn't need to be coloured red/green for added/removed lines.
The +/- & context lines also get individual style="color:FOO", rather than via a named style like everything else on the page.
Both of these unnecessarily increase the memory usage for the page (and I presume rendering time).
Assignee | ||
Comment 1•12 years ago
|
||
> Both of these unnecessarily increase the memory usage for the page (and I
> presume rendering time).
Which is very noticeable on pages larger than that given in comment 0, eg:
https://hg.mozilla.org/tracemonkey/rev/74b2b46fca7d
Comment 2•12 years ago
|
||
I assume this is these bits in the template:
http://hg.mozilla.org/hgcustom/hg_templates/file/672340227bea/gitweb_mozilla/map#l37
Assignee | ||
Comment 3•12 years ago
|
||
* Removes the unnecessary/empty <span>foo</span> on standard difflines.
* Adds styles for plus, minus and line number difflines to the stylesheet.
* Converts the difflineplus, difflineminus and difflineat entries to use these new styles.
Tested by modifying a locally saved copy of the page in comment 0; so as long as the templating system doesn't do something weird, should work fine :-)
(Amazing how productive one can be out of hours on a train, when not having to sheriff :-D)
Updated•12 years ago
|
Attachment #644408 -
Flags: review?(ted.mielczarek) → review+
Assignee | ||
Comment 4•12 years ago
|
||
Thank you :-)
I've just done some very rough testing with the changes made in this patch (by saving the page at the URL in comment 1 locally and making the same changes to the document/stylesheet).
tl;dr using the latest nightly, this reduces explicit memory usage from 1,132 MB to 784 MB for that page - and on my Core i7+8GB RAM+SSD, reduce page load time by ~10%. So pretty happy with the result :-)
Before:
1,131.69 MB (100.0%) -- explicit
├────945.83 MB (83.58%) -- window-objects
│ ├──939.69 MB (83.03%) -- top(file:///C:/Users/Ed/Desktop/hgweb%20optimisation/before/74b2b46fca7d.htm, id=8)/active/window(file:///C:/Users/Ed/Desktop/hgweb%20optimisation/before/74b2b46fca7d.htm)
│ │ ├──491.11 MB (43.40%) -- dom
│ │ │ ├──344.00 MB (30.40%) ── element-nodes
│ │ │ ├───86.82 MB (07.67%) ── text-nodes
│ │ │ ├───60.29 MB (05.33%) ── other [2]
│ │ │ └────0.00 MB (00.00%) ── comment-nodes
│ │ ├──448.31 MB (39.61%) -- layout
│ │ │ ├──224.92 MB (19.87%) ── style-contexts
│ │ │ ├──163.93 MB (14.49%) -- frames
│ │ │ │ ├───81.70 MB (07.22%) ── nsTextFrame
│ │ │ │ ├───81.59 MB (07.21%) ── nsInlineFrame
│ │ │ │ └────0.64 MB (00.06%) ++ (4 tiny)
│ │ │ ├───24.60 MB (02.17%) ── line-boxes
│ │ │ ├───17.45 MB (01.54%) ── pres-shell
│ │ │ ├───17.04 MB (01.51%) ── rule-nodes
│ │ │ └────0.37 MB (00.03%) ++ (3 tiny)
│ │ └────0.28 MB (00.02%) ++ (2 tiny)
│ └────6.14 MB (00.54%) ++ (5 tiny)
├────126.40 MB (11.17%) ── heap-unclassified
├─────32.17 MB (02.84%) ── history-links-hashtable
├─────19.01 MB (01.68%) -- js-non-window
│ ├──15.28 MB (01.35%) -- compartments
│ │ ├──14.06 MB (01.24%) ++ non-window-global
│ │ └───1.22 MB (00.11%) ++ no-global/compartment(atoms)
│ └───3.73 MB (00.33%) ++ (2 tiny)
└──────8.29 MB (00.73%) ++ (10 tiny)
After:
783.61 MB (100.0%) -- explicit
├──645.03 MB (82.31%) -- window-objects
│ ├──638.87 MB (81.53%) -- top(file:///C:/Users/Ed/Desktop/hgweb%20optimisation/after/74b2b46fca7d.htm, id=8)/active/window(file:///C:/Users/Ed/Desktop/hgweb%20optimisation/after/74b2b46fca7d.htm)
│ │ ├──455.05 MB (58.07%) -- dom
│ │ │ ├──307.94 MB (39.30%) ── element-nodes
│ │ │ ├───86.82 MB (11.08%) ── text-nodes
│ │ │ ├───60.29 MB (07.69%) ── other [2]
│ │ │ └────0.00 MB (00.00%) ── comment-nodes
│ │ ├──183.54 MB (23.42%) -- layout
│ │ │ ├──157.28 MB (20.07%) -- frames
│ │ │ │ ├───81.70 MB (10.43%) ── nsTextFrame
│ │ │ │ ├───74.95 MB (09.56%) ── nsInlineFrame
│ │ │ │ └────0.64 MB (00.08%) ++ (4 tiny)
│ │ │ ├───24.60 MB (03.14%) ── line-boxes
│ │ │ └────1.65 MB (00.21%) ++ (6 tiny)
│ │ └────0.28 MB (00.04%) ++ (2 tiny)
│ └────6.16 MB (00.79%) ++ (5 tiny)
├───70.62 MB (09.01%) ── heap-unclassified
├───32.17 MB (04.10%) ── history-links-hashtable
├───19.99 MB (02.55%) -- js-non-window
│ ├──15.25 MB (01.95%) -- compartments
│ │ ├──14.02 MB (01.79%) ++ non-window-global
│ │ └───1.22 MB (00.16%) ++ no-global/compartment(atoms)
│ └───4.74 MB (00.60%) ++ (2 tiny)
└───15.82 MB (02.02%) ++ (10 tiny)
Assignee | ||
Comment 5•12 years ago
|
||
Assignee | ||
Updated•12 years ago
|
Whiteboard: [sheriff-want]
Assignee | ||
Comment 6•12 years ago
|
||
Push to prod now fixed :-)
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Updated•11 years ago
|
Product: mozilla.org → Release Engineering
Comment 7•10 years ago
|
||
Updated•10 years ago
|
Product: Release Engineering → Developer Services
Comment 8•10 years ago
|
||
For posterity, I submitted these patches to upstream. http://www.selenic.com/pipermail/mercurial-devel/2015-January/065241.html
Assignee | ||
Comment 9•10 years ago
|
||
(In reply to Gregory Szorc [:gps] from comment #8)
> For posterity, I submitted these patches to upstream.
> http://www.selenic.com/pipermail/mercurial-devel/2015-January/065241.html
Thank you! :-)
Comment 10•10 years ago
|
||
(In reply to Gregory Szorc [:gps] from comment #8)
> For posterity, I submitted these patches to upstream.
> http://www.selenic.com/pipermail/mercurial-devel/2015-January/065241.html
It doesn't look like you took the bits that changed the inline style to a css class. Was that intentional?
Comment 11•10 years ago
|
||
I submitted it as 2 patches. CSS class patch is at http://www.selenic.com/pipermail/mercurial-devel/2015-January/065242.html
Mercurial *really* likes patches being as small as possible.
You need to log in
before you can comment on or make changes to this bug.
Description
•