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)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: emorley, Assigned: emorley)

References

Details

(Whiteboard: [sheriff-want])

Attachments

(1 file)

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).
> 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
Attached patch Patch v1 (deleted) — Splinter Review
* 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)
Assignee: nobody → bmo
Status: NEW → ASSIGNED
Attachment #644408 - Flags: review?(ted.mielczarek)
Attachment #644408 - Flags: review?(ted.mielczarek) → review+
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)
Depends on: 776874
Whiteboard: [sheriff-want]
Push to prod now fixed :-)
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Product: mozilla.org → Release Engineering
Product: Release Engineering → Developer Services
For posterity, I submitted these patches to upstream. http://www.selenic.com/pipermail/mercurial-devel/2015-January/065241.html
(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! :-)
(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?
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.

Attachment

General

Created:
Updated:
Size: