Remove unnecessary <span>s and per element styles on hgweb pages

RESOLVED FIXED

Status

Developer Services
Mercurial: hg.mozilla.org
RESOLVED FIXED
5 years ago
3 years ago

People

(Reporter: emorley, Assigned: emorley)

Tracking

Details

(Whiteboard: [sheriff-want])

Attachments

(1 attachment)

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

5 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
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

5 years ago
Created attachment 644408 [details] [diff] [review]
Patch v1

* 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+
(Assignee)

Comment 4

5 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

5 years ago
http://hg.mozilla.org/hgcustom/hg_templates/rev/aa5c47f0fb4f
(Assignee)

Updated

5 years ago
Depends on: 776874
(Assignee)

Updated

5 years ago
Whiteboard: [sheriff-want]
(Assignee)

Comment 6

5 years ago
Push to prod now fixed :-)
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Product: mozilla.org → Release Engineering

Comment 7

3 years ago
https://hg.mozilla.org/hgcustom/version-control-tools/rev/2c355a580af6
Product: Release Engineering → Developer Services

Comment 8

3 years ago
For posterity, I submitted these patches to upstream. http://www.selenic.com/pipermail/mercurial-devel/2015-January/065241.html
(Assignee)

Comment 9

3 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! :-)
(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.