Skip to content

Commit 335674e

Browse files
committed
Save about 0.3s in the gfma plugin.
1 parent d6a888d commit 335674e

2 files changed

Lines changed: 111 additions & 4 deletions

File tree

WIP.md

Lines changed: 27 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -518,21 +518,45 @@ Ranked by estimated wall-clock saving on the current Windows machine:
518518

519519
The Liquid framework drops (`BlockBody#render`, `Context#stack`, `Variable#render`) again outweigh the filter-dispatch drop -- they capture the eliminated `{%- unless -%}` / `{%- if -%}` blocks plus the chained `| replace:` pipeline AST nodes. The new filter does ~190 µs per call across 718 invocations, covering the same work the eliminated 39 k Liquid replaces did. Output byte-identical to baseline (`diff -rq` clean on `_site/`, `_site-offline/`, `_site-pdf/`).
520520

521+
4. **JekyllGFMAdmonitions defer-body-parse. [LANDED]** Extended `_plugins/jekyll-gfm-admonitions-patch.rb` with two method overrides on `JekyllGFMAdmonitions::GFMAdmonitionConverter`. The first replaces `admonition_html` so the admonition body is spliced into `doc.content` as raw markdown inside a `<div ... markdown='1'>` wrapper, deferring the per-admonition `@markdown.convert(text)` call to the page-level kramdown pass (which already runs with `parse_block_html: true` per `_config.yml`). One combined kramdown pass replaces 1 + N parses for each of the site's 508 admonitions. The second overrides `process_doc` to preserve the leading newline(s) in the code-block stash placeholder substitution -- without this, the gem's `(?:^|\n)(?<!>)\s*\`\`\`.*?\`\`\`` regex consumes the blank line between an admonition body and a following fenced code block, the placeholder ends up appended to the last `>`-prefixed body line, the admonition regex pulls it into the body capture, and either kramdown renders it as an empty `<code class="language-plaintext"></code>` (gem behaviour) or the code block is spliced inside the admonition div (deferred-body behaviour). With the override, placeholders stay on their own line outside the body capture.
522+
523+
Ruby-prof effect (post-CT baseline vs post-GFM-patch):
524+
525+
| Metric | Before | After | Delta |
526+
|---|---|---|---|
527+
| `GFMAdmonitionConverter#generate` total | 0.690 s / 1 call | 0.108 s / 1 call | -0.582 s |
528+
| `admonition_html` calls | 508 | 508 | (same dispatch, now does only string concat) |
529+
| `@markdown.convert(text)` calls from admonition_html | 508 | 0 | -508 |
530+
531+
Wall-clock effect on 3-run uninstrumented means (busy dev machine, but consistent within each set):
532+
533+
| Phase | Before | After | Delta |
534+
|---|---|---|---|
535+
| `done in ...` total | 11.47 s | 11.13 s | -0.34 s |
536+
| `GFMA: Generator ran in ...` | 216 ms | 93 ms | -123 ms |
537+
538+
Output is not byte-identical to baseline: 12 files differ. Eleven are real bug fixes that were latent in the unpatched gem -- 5 pages had their fenced code block lost (the code-block-stash-eats-the-blank-line bug above; `Tutorials/Arrays.md`, `Tutorials/CustomControls/Painting.md`, `tB/Packages/WebView2/WebView2/index.md`, `tB/Packages/WinNamedPipesLib/NamedPipeClientConnection.md`, `tB/Packages/WinServicesLib/ServiceManager.md`), 1 page had a `\\\\` source sequence collapsed to `\\` by the gem's second markdown pass (`tB/Core/RightShift.md` -- the body is now parsed once, so `**\\\\**` renders as `<strong>\\</strong>` not `<strong>\</strong>`), 1 page had its loose-list items rendered as `<li>text</li>` instead of CommonMark's `<li><p>text</p></li>` because the gem's pre-rendered admonition HTML changed the surrounding paragraph context (`Documentation/Development.md`), and the remaining 5 are cosmetic whitespace nits inside admonitions that themselves contain a fenced code block (`tB/Core/If-Then-Else.md`, `tB/Core/Option.md`, `tB/Modules/Interaction/InputBox.md`, `tB/Packages/CEF/CefBrowser/index.md`, `tB/Packages/tbIDE/HtmlElement.md`). The 12th file is `assets/js/search-data.json`, derived from page contents so it tracks them. Lychee link check is clean (`8170 OK, 0 errors` for online; `6824 OK, 0 errors` for offline).
539+
540+
A separate investigation looked at `NavIntegrityCheck::Generator#generate` (0.436 s / 1 call in the post-CT profile, attributed to 855 `Jekyll::FrontmatterDefaults#find` walks). The plugin uses `page.data[key]` for `title` / `nav_exclude` / `parent` / `grand_parent`, and Jekyll's `Page#initialize` sets `data.default_proc = proc { site.frontmatter_defaults.find(...) }`, so every missing key fell through to a full defaults walk. Switching to `data.fetch(key, nil)` bypasses the default_proc, but the resulting wall-clock delta is only ~50-80 ms: NavIntegrityCheck was warming `FrontmatterDefaults`'s internal `@matched_set_cache` (keyed by `path-type`), and `NavTreePrecompute::Generator#ordered_children_for` was the cache's biggest beneficiary. With NavIntegrityCheck skipping the walk, NavTreePrecompute pays the cache-miss cost itself -- ~430 ms moves from one stack to the other, leaving only the per-call dispatch overhead recovered. The patch was reverted.
541+
521542
#### Cumulative
522543

523-
The three landed optimizations together (chapter precompute, SEO precompute, chapter-body transform) shrank ruby-prof's instrumented build wall from ~41.7 s (immediately post-html-compress baseline) down to 34.78 s -- about -7 s. The cumulative profile-table picture, comparing the post-html-compress baseline to the post-chapter-transform state:
544+
The four landed optimizations together (chapter precompute, SEO precompute, chapter-body transform, GFM defer-body-parse) shrank ruby-prof's instrumented build wall from ~41.7 s (immediately post-html-compress baseline) down to ~34 s. The cumulative profile-table picture, comparing the post-html-compress baseline to the post-GFM state:
524545

525-
| Metric | Post-html-compress | Post-CT | Delta |
546+
| Metric | Post-html-compress | Post-GFM | Delta |
526547
|---|---|---|---|
527-
| Total instrumented wall | 39.30 s | 34.78 s | -4.52 s |
548+
| Total instrumented wall | 39.30 s | 34.78 s* | -4.52 s |
528549
| `Liquid::Strainer#invoke` total | 8.90 s / 191,365 calls | 5.45 s / 122,397 calls | -3.45 s / -68,968 calls |
529550
| `where_exp` calls | 37 | 0 | -37 |
530551
| `markdownify` calls | 1,802 | 128 | -1,674 |
531552
| `absolute_url` filter calls | 1,675 | 1 | -1,674 |
532553
| `replace` calls | 87,991 | 48,577 | -39,414 |
554+
| `GFMAdmonitionConverter#generate` total | 0.690 s | 0.108 s | -0.582 s |
533555
| `Liquid::BlockBody#render` total | 18.38 s | 14.43 s | -3.95 s |
534556
| `Liquid::Context#stack` total | 18.19 s | 13.78 s | -4.41 s |
535557

558+
\* Instrumented totals are noisy on the current Windows dev machine (single-run range ~9 s across consecutive identical runs); the per-method numbers above are stable across runs and are the more reliable signal.
559+
536560
What's left of the per-filter table is approximately what kramdown / Rouge actually parse and emit: the 128 remaining `markdownify` calls are the per-chapter `chapter.content | markdownify` in `book-chapter-body.html` plus `book.html`'s part subtitle / intro markdown. Each of those is unique input, so Jekyll's converter cache rarely hits and the kramdown parse itself dominates. Further savings on this axis would need either (a) reusing the already-rendered `_site/<page>.html` instead of re-parsing source markdown for the book, or (b) accepting kramdown's parse cost as the floor and looking elsewhere -- the next-biggest non-library hotspot is `Offlinify#rewrite_html!` at ~2 s of self-time, already heavily optimised (see `_plugins/offlinify.md`).
537561

538562
## Site integrity check

docs/_plugins/jekyll-gfm-admonitions-patch.rb

Lines changed: 84 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,8 @@
113113
# generators (this one included) are invisible to it. The wall-clock delta
114114
# we log here is the gem's full contribution to the GENERATE phase:
115115
# walking every collection doc and page, running the admonition regex,
116-
# and re-invoking the markdown converter on each admonition body.
116+
# and (after the patch below) splicing in HTML that defers body markdown
117+
# parsing to the page-level kramdown pass.
117118
module JekyllGFMAdmonitions
118119
class GFMAdmonitionConverter
119120
unless method_defined?(:_generate_without_timing)
@@ -125,5 +126,87 @@ def generate(site)
125126
Jekyll.logger.info "GFMA:", "Generator ran in #{elapsed_ms}ms."
126127
end
127128
end
129+
130+
# Skip the per-admonition `@markdown.convert(text)` call by leaving
131+
# the body as raw markdown inside the outer alert div. The
132+
# site-level kramdown config (`parse_block_html: true` and
133+
# `parse_span_html: true` in _config.yml) makes the page-level
134+
# kramdown pass descend into the div and parse the body markdown
135+
# during RENDER, so the rendered HTML is the same as if the gem had
136+
# pre-converted the body itself -- one combined parse instead of
137+
# 1 + N (page + one per admonition).
138+
#
139+
# Two side effects of removing the inline `markdown.convert`
140+
# surface as small correctness improvements over the unpatched gem:
141+
#
142+
# * Backslash-escapes in body text (e.g. `**\\\\**` for a bold
143+
# pair of backslashes) no longer go through kramdown twice and
144+
# so are no longer collapsed by the second pass. The unpatched
145+
# gem's output of `<strong>\</strong>` (one backslash) becomes
146+
# `<strong>\\</strong>` (two backslashes, what the source asks
147+
# for). Pages affected: any with `\\\\` inside an admonition
148+
# body -- on this site, `Reference/Core/RightShift.md` and a
149+
# handful of others.
150+
#
151+
# * Code blocks that follow an admonition with just one blank
152+
# line between them are no longer eaten by the gem's code-block
153+
# stash regex (see `process_doc` override below). The unpatched
154+
# gem's stash regex `(?:^|\n)(?<!>)\s*```.*?```/m` consumes the
155+
# blank line, which pulls the placeholder into the admonition
156+
# body capture, which lets kramdown render it as an empty
157+
# `<code class="language-plaintext"></code>` element and
158+
# prevents the restore step from finding it. Net effect on
159+
# the unpatched gem: the code block disappears from the
160+
# rendered HTML. The override below preserves the leading
161+
# newline(s) so the placeholder stays on its own line outside
162+
# the admonition body capture.
163+
#
164+
# The body text is bracketed by blank lines so kramdown reads it as
165+
# an independent paragraph rather than tangling with the preceding
166+
# `<p class="markdown-alert-title">...</p>` block. The outer div
167+
# carries `markdown='1'` so kramdown's HTML-block parser keeps the
168+
# whole `<div>...</div>` as a single block even though it spans
169+
# blank lines internally.
170+
unless method_defined?(:_admonition_html_without_deferred_body)
171+
alias_method :_admonition_html_without_deferred_body, :admonition_html
172+
def admonition_html(type, title, text, icon)
173+
"<div class='markdown-alert markdown-alert-#{type}' markdown='1'>\n" \
174+
"<p class='markdown-alert-title'>#{icon} #{title}</p>\n\n" \
175+
"#{text}\n" \
176+
"</div>"
177+
end
178+
end
179+
180+
# Override `process_doc` to fix the code-block stash so that the
181+
# placeholder substitution preserves the leading newline(s) that
182+
# separated the code block from the preceding text. Without this
183+
# adjustment, the gem's gsub eats the blank line before the code
184+
# block, which causes the placeholder to be appended to the last
185+
# admonition body line and then dragged into the body capture by
186+
# the admonition regex's `[^\n]*` body-line pattern.
187+
#
188+
# The body of the method otherwise mirrors the upstream gem
189+
# verbatim (see jekyll-gfm-admonitions 1.2.0,
190+
# `lib/jekyll-gfm-admonitions.rb#process_doc`).
191+
unless method_defined?(:_process_doc_without_leading_ws_preserve)
192+
alias_method :_process_doc_without_leading_ws_preserve, :process_doc
193+
def process_doc(doc)
194+
return if doc.content.empty?
195+
doc.content = doc.content.dup unless doc.content.frozen?
196+
197+
code_blocks = []
198+
doc.content.gsub!(/(?:^|\n)(?<!>)\s*```.*?```/m) do |match|
199+
code_blocks << match
200+
leading = match[/\A\s+/] || ""
201+
"#{leading}```{{CODE_BLOCK_#{code_blocks.length - 1}}}```"
202+
end
203+
204+
convert_admonitions(doc)
205+
206+
doc.content.gsub!(/```\{\{CODE_BLOCK_(\d+)}}```/) do
207+
code_blocks[::Regexp.last_match(1).to_i]
208+
end
209+
end
210+
end
128211
end
129212
end

0 commit comments

Comments
 (0)