Improve build time of navigation panel (#956)

Fix #863.

The current Liquid code to generate the navigation panel involves the inefficient extraction of a list of pages from a list of page groups (identified by @captn3m0 in his original [explanation of the performance issue](https://github.com/just-the-docs/just-the-docs/issues/863)). 

The optimisation implemented by this PR generates navigation links directly from the list of page groups, thereby avoiding the extraction of a list of pages from it. The Liquid code is now a bit tedious, but I don't see a simpler solution. 

The need for grouping pages arises because Jekyll doesn't provide a filter to sort a list of pages on the value of an arbitrary expression.

Using Jekyll v4.2.2 (macOS 12.5, M2 MacBook Air, 16 GB memory), building https://github.com/endoflife-date/endoflife.date using https://github.com/pdmosses/just-the-docs/blob/fix-nav-performance/_includes/nav.html produced the following profile extract:

Filename                                                    | Count |    Bytes |   Time
------------------------------------------------------------|-------|----------|-------
| just-the-docs-0.4.0.rc1/_layouts/default.html                    |   130 |  3792.04K |  5.160 |
| _includes/nav.html                                               |   130 |  1405.20K |  4.054 |
| just-the-docs-0.4.0.rc1/_includes/head.html                      |   130 |   617.82K |  0.495 |
| _layouts/product.html                                            |   127 |  1014.38K |  0.413 |
| _includes/head_custom.html                                       |   130 |   427.83K |  0.393 |
| assets/js/zzzz-search-data.json                                  |     1 |   149.31K |  0.050 |

@nathancarter has tried adding the new `nav.html` to [a site with over 300 pages](https://nathancarter.github.io/how2data/site/), and reported that it improved the build time of more than 3 minutes to about 30 seconds.

Further optimisation of navigation might be possible (e.g., using [Jekyll include caching](https://github.com/benbalter/jekyll-include-cache)), but the current optimisation should be sufficient for v0.4.0.

To test that this PR does not appear to affect the navigation panel generated by v0.3.3:

1. Clone https://github.com/just-the-docs/just-the-docs-tests.
2. Update `_config.yml` and `Gemfile` to use this PR branch.
3. Run `bundle update`.
4. Inspect the rendering of the entire collection of navigation tests.

(Many of the differences reported in the GitHub visualisation of the changes are due to shifting much of the code 2 spaces to the left, in connection with moving the first `ul` element to be close to its first item.)
This commit is contained in:
Peter Mosses 2022-09-12 05:31:17 +02:00 committed by GitHub
parent 03e1db9506
commit 457dce3651
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23

View File

@ -1,8 +1,6 @@
<ul class="nav-list">
{%- assign titled_pages = include.pages
| where_exp:"item", "item.title != nil" -%}
{%- comment -%}
Pages with no `title` are implicitly excluded from the navigation.
The values of `title` and `nav_order` can be numbers or strings.
Jekyll gives build failures when sorting on mixtures of different types,
so numbers and strings need to be sorted separately.
@ -14,19 +12,25 @@
The case-sensitivity of string sorting is determined by `site.nav_sort`.
{%- endcomment -%}
{%- assign titled_pages = include.pages
| where_exp: "item", "item.title != nil" -%}
{%- assign string_ordered_pages = titled_pages
| where_exp: "item", "item.nav_order == nil" -%}
{%- assign nav_ordered_pages = titled_pages
| where_exp: "item", "item.nav_order != nil" -%}
{%- comment -%}
The nav_ordered_pages have to be added to number_ordered_pages and
string_ordered_pages, depending on the nav_order value.
The first character of the jsonify result is `"` only for strings.
Add the nav-ordered pages to the number-ordered pages or the string-ordered pages,
depending on their `nav_order` value.
The first character of the `jsonify` result is `"` only for strings.
{%- endcomment -%}
{%- assign nav_ordered_groups = nav_ordered_pages
| group_by_exp: "item", "item.nav_order | jsonify | slice: 0" -%}
{%- assign number_ordered_pages = "" | split:"X" -%}
{%- assign number_ordered_pages = "" | split: "" -%}
{%- for group in nav_ordered_groups -%}
{%- if group.name == '"' -%}
{%- assign string_ordered_pages = string_ordered_pages | concat: group.items -%}
@ -35,29 +39,33 @@
{%- endif -%}
{%- endfor -%}
{%- assign sorted_number_ordered_pages = number_ordered_pages | sort:"nav_order" -%}
{%- assign sorted_number_ordered_groups = number_ordered_pages
| sort: "nav_order" | group_by: "nav_order" -%}
{%- comment -%}
The string_ordered_pages have to be sorted by nav_order, and otherwise title
(where appending the empty string to a numeric title converts it to a string).
After grouping them by those values, the groups are sorted, then the items
of each group are concatenated.
Group the string-ordered pages by `nav_order`, if non-nil, and otherwise `title`
(but appending the empty string to a numeric title to convert it to a string).
Then sort the groups according to the site setting for case (in)sensitivity.
{%- endcomment -%}
{%- assign string_ordered_groups = string_ordered_pages
| group_by_exp:"item", "item.nav_order | default: item.title | append: '' " -%}
{%- if site.nav_sort == 'case_insensitive' -%}
{%- assign sorted_string_ordered_groups = string_ordered_groups | sort_natural:"name" -%}
{%- assign sorted_string_ordered_groups = string_ordered_groups
| sort_natural: "name" -%}
{%- else -%}
{%- assign sorted_string_ordered_groups = string_ordered_groups | sort:"name" -%}
{%- assign sorted_string_ordered_groups = string_ordered_groups
| sort:"name" -%}
{%- endif -%}
{%- assign sorted_string_ordered_pages = "" | split:"X" -%}
{%- for group in sorted_string_ordered_groups -%}
{%- assign sorted_string_ordered_pages = sorted_string_ordered_pages | concat: group.items -%}
{%- endfor -%}
{%- assign pages_list = sorted_number_ordered_pages | concat: sorted_string_ordered_pages -%}
{%- assign groups_list = sorted_number_ordered_groups
| concat: sorted_string_ordered_groups -%}
{%- for node in pages_list -%}
<ul class="nav-list">
{%- for node_group in groups_list -%}
{%- for node in node_group.items -%}
{%- if node.parent == nil -%}
{%- unless node.nav_exclude -%}
<li class="nav-list-item{% if page.collection == include.key and page.url == node.url or page.parent == node.title or page.grand_parent == node.title %} active{% endif %}">
@ -68,10 +76,15 @@
{%- endif -%}
<a href="{{ node.url | relative_url }}" class="nav-list-link{% if page.url == node.url %} active{% endif %}">{{ node.title }}</a>
{%- if node.has_children -%}
{%- assign children_list = "" | split: "" -%}
{%- for parent_group in groups_list -%}
{%- assign children_list = children_list
| concat: parent_group.items
| where: "parent", node.title
| where_exp:"item", "item.grand_parent == nil" -%}
{%- endfor -%}
{%- if node.child_nav_order == 'desc' -%}
{%- assign children_list = pages_list | where: "parent", node.title | where_exp:"item", "item.grand_parent == nil" | reverse -%}
{%- else -%}
{%- assign children_list = pages_list | where: "parent", node.title | where_exp:"item", "item.grand_parent == nil" -%}
{%- assign children_list = children_list | reverse -%}
{%- endif -%}
<ul class="nav-list ">
{%- for child in children_list -%}
@ -84,16 +97,21 @@
{%- endif -%}
<a href="{{ child.url | relative_url }}" class="nav-list-link{% if page.url == child.url %} active{% endif %}">{{ child.title }}</a>
{%- if child.has_children -%}
{%- assign grandchildren_list = "" | split: "" -%}
{%- for grandparent_group in groups_list -%}
{%- assign grandchildren_list = grandchildren_list
| concat: grandparent_group.items
| where: "parent", child.title
| where: "grand_parent", node.title -%}
{%- endfor -%}
{%- if node.child_nav_order == 'desc' -%}
{%- assign grand_children_list = pages_list | where: "parent", child.title | where: "grand_parent", node.title | reverse -%}
{%- else -%}
{%- assign grand_children_list = pages_list | where: "parent", child.title | where: "grand_parent", node.title -%}
{%- assign grandchildren_list = grandchildren_list | reverse -%}
{%- endif -%}
<ul class="nav-list">
{%- for grand_child in grand_children_list -%}
{%- unless grand_child.nav_exclude -%}
<li class="nav-list-item {% if page.url == grand_child.url %} active{% endif %}">
<a href="{{ grand_child.url | relative_url }}" class="nav-list-link{% if page.url == grand_child.url %} active{% endif %}">{{ grand_child.title }}</a>
{%- for grandchild in grandchildren_list -%}
{%- unless grandchild.nav_exclude -%}
<li class="nav-list-item {% if page.url == grandchild.url %} active{% endif %}">
<a href="{{ grandchild.url | relative_url }}" class="nav-list-link{% if page.url == grand_child.url %} active{% endif %}">{{ grandchild.title }}</a>
</li>
{%- endunless -%}
{%- endfor -%}
@ -108,6 +126,7 @@
{%- endunless -%}
{%- endif -%}
{%- endfor -%}
{%- endfor -%}
{%- assign nav_external_links = site.nav_external_links -%}
{%- for node in nav_external_links -%}
<li class="nav-list-item external">
@ -121,16 +140,28 @@
{%- if page.collection == include.key -%}
{%- for node in pages_list -%}
{%- for node_group in groups_list -%}
{%- for node in node_group.items -%}
{%- if node.parent == nil -%}
{%- if page.grand_parent == node.title or page.parent == node.title and page.grand_parent == nil -%}
{%- if page.grand_parent == node.title
or page.parent == node.title
and page.grand_parent == nil -%}
{%- assign first_level_url = node.url | relative_url -%}
{%- endif -%}
{%- if node.has_children -%}
{%- assign children_list = pages_list | where: "parent", node.title -%}
{%- assign children_list = "" | split: "" -%}
{%- for parent_group in groups_list -%}
{%- assign children_list = children_list | concat:
parent_group.items | where: "parent", node.title -%}
{%- endfor -%}
{%- if node.child_nav_order == 'desc' -%}
{%- assign children_list = children_list | reverse -%}
{%- endif -%}
{%- for child in children_list -%}
{%- if child.has_children -%}
{%- if page.url == child.url or page.parent == child.title and page.grand_parent == child.parent -%}
{%- if page.url == child.url
or page.parent == child.title
and page.grand_parent == child.parent -%}
{%- assign second_level_url = child.url | relative_url -%}
{%- endif -%}
{%- endif -%}
@ -138,9 +169,19 @@
{%- endif -%}
{%- endif -%}
{%- endfor -%}
{%- endfor -%}
{% if page.has_children == true and page.has_toc != false %}
{%- assign toc_list = pages_list | where: "parent", page.title | where: "grand_parent", page.parent -%}
{%- assign toc_list = "" | split: "" -%}
{%- for parent_group in groups_list -%}
{%- assign toc_list = toc_list
| concat: parent_group.items
| where: "parent", page.title
| where: "grand_parent", page.parent -%}
{%- endfor -%}
{%- if node.child_nav_order == 'desc' -%}
{%- assign toc_list = toc_list | reverse -%}
{%- endif -%}
{%- endif -%}
{%- endif -%}