Optimize the element selectors in the diff viewer.

Review Request #8891 — Created April 10, 2017 and submitted

Christian Hammond
Review Board

Rendered diffs contain a lot of elements, and at times (during setup,
navigation, and resize handling, we have to query for specific elements.
Some of the queries we had were very inefficient, impacting these
operations. Most users wouldn't have noticed this, but there is a
measurable impact that can cause real issues with very large diffs.

One of the problems had to do with the use of the :first filter for
jQuery selectors. This filter is equivalent to grabbing the first result
from a query, but does not stop the query at the first result. We can
querySelector() instead to get the result we want. This makes a huge

Similarly, our parent element lookups now use closest(), instead of
parents(), giving us another performance boost.

Queries for anchors with a given name now use
document.getElementsByName(), which is much faster. This improves anchor

Tested all forms of navigation:

  • Clicking chunks
  • Keyboard bindings
  • Chunk and file anchors in the diff index
  • Hashes in the URL (for files, chunks, and line numbers)

Tested expanding/collapsing chunks.

Measured performance for all the selectors using large diffs, and saw
noticeable decreases in time.

  • 0
  • 0
  • 2
  • 0
  • 2
Description From Last Updated
David Trowbridge
  2. .revision-row and .filename-row are direct children of <thead> so this can use $thead.children(...) to make it even slightly better.

  3. Can we reformat this a little?

    cellPadding = $(this.el.querySelector('pre'))
        .getExtents('p', 'lr');
Christian Hammond
David Trowbridge
  1. Ship It!
Christian Hammond
Review request changed

Status: Closed (submitted)

Change Summary:

Pushed to release-2.5.x (54bc233)