From 457dce36515d454228b7504bfd57ab92131c6dad Mon Sep 17 00:00:00 2001
From: Peter Mosses <18308236+pdmosses@users.noreply.github.com>
Date: Mon, 12 Sep 2022 05:31:17 +0200
Subject: [PATCH] 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.)
---
_includes/nav.html | 175 ++++++++++++++++++++++++++++-----------------
1 file changed, 108 insertions(+), 67 deletions(-)
diff --git a/_includes/nav.html b/_includes/nav.html
index 9d322dc3..53838bc9 100644
--- a/_includes/nav.html
+++ b/_includes/nav.html
@@ -1,63 +1,71 @@
-<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.
- {%- comment -%}
- 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.
+ Here, numbers are sorted by their values, and come before all strings.
+ An omitted `nav_order` value is equivalent to the page's `title` value
+ (except that a numerical `title` value is treated as a string).
- Here, numbers are sorted by their values, and come before all strings.
- An omitted `nav_order` value is equivalent to the page's `title` value
- (except that a numerical `title` value is treated as a string).
+ The case-sensitivity of string sorting is determined by `site.nav_sort`.
+{%- endcomment -%}
- The case-sensitivity of string sorting is determined by `site.nav_sort`.
- {%- endcomment -%}
-
- {%- 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.
- {%- endcomment -%}
- {%- assign nav_ordered_groups = nav_ordered_pages
- | group_by_exp:"item", "item.nav_order | jsonify | slice: 0" -%}
- {%- assign number_ordered_pages = "" | split:"X" -%}
- {%- for group in nav_ordered_groups -%}
- {%- if group.name == '"' -%}
- {%- assign string_ordered_pages = string_ordered_pages | concat: group.items -%}
- {%- else -%}
- {%- assign number_ordered_pages = number_ordered_pages | concat: group.items -%}
- {%- endif -%}
- {%- endfor -%}
-
- {%- assign sorted_number_ordered_pages = number_ordered_pages | sort:"nav_order" -%}
+{%- 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 -%}
+ Add the nav-ordered pages to the number-ordered pages or the string-ordered pages,
+ depending on their `nav_order` value.
- {%- 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.
- {%- 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" -%}
+ 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: "" -%}
+{%- for group in nav_ordered_groups -%}
+ {%- if group.name == '"' -%}
+ {%- assign string_ordered_pages = string_ordered_pages | concat: group.items -%}
{%- else -%}
- {%- assign sorted_string_ordered_groups = string_ordered_groups | sort:"name" -%}
+ {%- assign number_ordered_pages = number_ordered_pages | concat: group.items -%}
{%- 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 -%}
+{%- endfor -%}
+
+{%- assign sorted_number_ordered_groups = number_ordered_pages
+ | sort: "nav_order" | group_by: "nav_order" -%}
- {%- assign pages_list = sorted_number_ordered_pages | concat: sorted_string_ordered_pages -%}
+{%- comment -%}
+ 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).
- {%- for node in pages_list -%}
+ 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" -%}
+{%- else -%}
+ {%- assign sorted_string_ordered_groups = string_ordered_groups
+ | sort:"name" -%}
+{%- endif -%}
+
+{%- assign groups_list = sorted_number_ordered_groups
+ | concat: sorted_string_ordered_groups -%}
+
+<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 -%}
--
GitLab