Opened 5 years ago

Closed 5 years ago

#11563 closed enhancement (fixed)

Improve consensus-health page by reducing size of HTML

Reported by: alphawolf Owned by: karsten
Priority: Medium Milestone:
Component: Metrics/Website Version:
Severity: Keywords: consensus-health
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

Problem:

The consensus-health page @ https://consensus-health.torproject.org/ is currently ~ 5.4 MB uncompressed (231 KB over the wire, using gzip). The large size of the uncompressed document makes browser rending take longer than it needs to, consumes excess RAM, and (probably) consumes excess CPU cycles on the server during compression, or excess bandwidth when served uncompressed.

Specifics:

  1. Redundant content:
    1. 25,862 instances of '<font color="blue">(.*?)</font>'
    2. 5,068 instances of '<font color="gray"><s>(.*?)</s></font>'
    3. 2,200 instances of '<font color="red">(.*?)</font>'
  2. Unnecessary elements:
    1. Headers in relay list use '<b>(.*?)<b/>' for bolding when changing the '<td>' to '<th>' would give the same presentation for less bytes, while being more semantically correct HTML. (8,256 instances)
    2. Headers in relay list use '<br>' before each DirAuth name in order to provide spacing (8,256 instances)
  3. Unnecessary white space:
    1. 572,662 instances of <space><space>
    2. 97,899 instances of '\n' between (not inside) tags.

Proposal:

Implement the following changes, which reduce the document size without affecting its presentation:

  1. Reduce redundant content:
    1. Remove '<font color="blue"></font>' for each individual flag, and set ' class="ic"' on the column instead. Add appropriate CSS. Estimated savings: 600 KB
    2. Replace '<font color="gray"><s></s></font>' with '<span class="oic"><span>'. Add appropriate CSS. Estimated savings: 40 KB
    3. Replace '<font color="red"></font>' with '<span class="oiv"></span>'. Add appropriate CSS. Estimated savings: This is actually a small net loss, but by changing <font> to <span> it will match the other changes and compress more efficiently.
  2. Remove unnecessary elements:
    1. Remove all '<b></b>' and '<br>' tags from the relay list headers. Set ' class="tbl-hdr"' on the <tr> and change '<td>' to '<th>'. Add height and vertical alignment to CSS. Estimated savings: 77 KB
  3. Remove unnecessary white space:
    1. <space><space>
      1. Replace each <space><space> with "\t". Estimated savings: 570 KB
      2. OR: Remove all instances of <space><space> entirely. Estimated savings: 1.1 MB
      3. OR: Replace the white space with a variable that can be enabled when white space is desired (for debuging purposes). Estimated savings: 1.1 MB
    2. newline
      1. Remove all newlines after block/table-related elements. Estimated savings: 95 KB
      2. OR: Replace newlines with variable that can be enabled for debugging purposes. Estimated savings: 95 KB

By implementing the above changes, the consensus-relay could be trimmed by up to 1.9 MB -- a full 1/3 of it's 5.4 MB bulk. Since part 3 could be controversial, I'll put that in a separate patch (assuming it is wanted). I'm attaching a patch that implements parts 1 and 2. It should trim the uncompressed document by around 700 KB.

Child Tickets

Attachments (5)

ticket_11563.patch (15.9 KB) - added by alphawolf 5 years ago.
Patch, parts 1 and 2
ticket_11563-2.patch (44.5 KB) - added by alphawolf 5 years ago.
Implements part 3
relay-list-before (1.5 KB) - added by alphawolf 5 years ago.
Ex. HTML before part 4 change
relay-list-after (1.1 KB) - added by alphawolf 5 years ago.
Ex. HTML after part 4 change
0001-Replace-br-with-newline-in-relay-list-and-format-wit.patch (1.8 KB) - added by alphawolf 5 years ago.
Patch, part 4

Download all attachments as: .zip

Change History (16)

Changed 5 years ago by alphawolf

Attachment: ticket_11563.patch added

Patch, parts 1 and 2

comment:1 Changed 5 years ago by alphawolf

Please note, the patch needs tested.  While I was very careful and tested the changes on the static HTML, I am unable to test the actual changes to the java source.

comment:2 Changed 5 years ago by karsten

Thanks, I applied your patch. Looks good so far.

How about we take out leading spaces and leave newlines unchanged? That seems like the best tradeoff between saving bytes and keeping the source readable for humans. Want to write another patch for that?

Changed 5 years ago by alphawolf

Attachment: ticket_11563-2.patch added

Implements part 3

comment:3 Changed 5 years ago by alphawolf

Agreed on the leading spaces. Patch submitted, but needs tested.

I have an idea for a "part 4", but I'll need to test it in HTML/CSS first. There are over 240K '<br>' tags, mostly between the flags in the relay list. I believe these could be replaced with a single newline each, and then CSS applied to make those newlines actually render. (In the relay table only). This change would shave off another 720 KB.

The downside of "part 4" is that it would be a little harder for humans to read the HTML source, as each flag in source view would be on its own line. Thoughts?

comment:4 Changed 5 years ago by atagar

Component: DocTorMetrics Website

Reassigning the component since DocTor now just consists of the tor-consensus-health@ alerts. The consensus health page lives separately as part of the metrics site nowadays.

comment:5 Changed 5 years ago by atagar

Owner: changed from atagar to karsten
Status: newassigned

comment:6 Changed 5 years ago by alphawolf

Part 4 achieves significant gains in render time, especially in Firefox.  After replacing '<br>' with '\n', document renders in a pretty consistent 3-4 seconds under Firefox.  Before the change, it renders in 5-9 seconds.  Under Chrome, DOMContentLoaded is cut from 3.4 seconds to 2.4.  File is reduced to about 2.9 MB.  Not bad, starting from 5.4 MB!  I'll supply a patch, if desired.

comment:7 Changed 5 years ago by karsten

Applied the second patch, looks good!

To be honest, I'm not clear how the patch for your part 4 would look like. Do you mind attaching a diff of the HTML before and after that change? Thanks!

comment:8 Changed 5 years ago by alphawolf

I don't think a diff will be very decipherable, because the HTML is too different.  I'll attach two small samples to show how the HTML differs.  They will give identical presentations with the proper CSS.

The files contain the same two rows of relay data:

Changed 5 years ago by alphawolf

Attachment: relay-list-before added

Ex. HTML before part 4 change

Changed 5 years ago by alphawolf

Attachment: relay-list-after added

Ex. HTML after part 4 change

Changed 5 years ago by alphawolf

Patch, part 4

comment:9 Changed 5 years ago by alphawolf

I went ahead and made a part 4 patch, since it is actually a remarkably small amount of code that gets changed.  If you like it, great, -- if not, I won't be offended. :)   I understand that the readability of the HTML suffers perhaps more than the performance boost warrants.

comment:10 Changed 5 years ago by karsten

Merged and deployed. If there are no further low-langing fruit left, I'd say let's resolve this ticket. Thanks for your efforts here!

Slightly off-topic: If you like consensus-health, you might also be interested in #9778 and proposal 164. The goal would be to include more information in Onionoo responses and then either add those to Globe or build a new service similar to consensus-health. If you want to help move these things forward, please let me know.

comment:11 Changed 5 years ago by alphawolf

Resolution: fixed
Status: assignedclosed

Sounds good. Closing the ticket. Thanks for your help here as well!

I'll take a look at the others and see if it's something I can work on.

Note: See TracTickets for help on using tickets.