Skip to content

Free the HTML parser after each parse instead of keeping one per thread - #49

Open
ilyazub wants to merge 2 commits into
masterfrom
free-html-parser-after-parse
Open

ilyazub wants to merge 2 commits into
masterfrom
free-html-parser-after-parse

Conversation

@ilyazub

@ilyazub ilyazub commented Oct 15, 2026 •

Copy link
Copy Markdown

AI-generated: Claude Opus 5.5, high effort: code review, tests and benchmarks (wrote commit fcf1a89 and this text; 2dff075 came from an earlier session of the same work and was reviewed here).

Nokolexbor::HTML now creates the Lexbor HTML parser for each parse and destroys it when the document is built, instead of caching one per thread in a pthread key. A server whose thread pool parses large pages no longer keeps each thread's parser buffers after the documents are gone: about 22 MB less allocated memory with 16 threads.

The cached parser was destroyed only at thread exit, so in a long-lived pool (Puma, Sidekiq) every thread kept tokenizer and tree buffers sized for the biggest page it had parsed. The non-pthread build already created and destroyed the parser per parse; this makes every platform do that. The CSS parser and selector caches stay per thread.

Memory. 16 threads parse 4,000 real Google result pages (8 pages, 0.3-1.4 MB), drop the documents, then stay parked like a server pool; measured after GC.start x3 with jemalloc (narenas:1) stats.allocated, 3 interleaved rounds per arm [min-max]:

Ruby master 0375a7b this PR change
2.7.8 77.9 MB [77.7-78.7] 55.2 MB [54.5-55.6] −22.7 MB
3.3.10 61.0 MB [60.0-61.2] 38.7 MB [38.3-39.2] −22.3 MB
4.0.7 60.9 MB [60.3-61.6] 39.0 MB [38.9-39.2] −21.9 MB

jemalloc stats.resident moves the same way but is noisy on this machine: 2.7.8 157.6-186.7 → 131.3-147.7 MB; 3.3.10 181.2-237.3 → 164.0-186.2 MB (ranges overlap); 4.0.7 295.5-364.1 → 282.8-291.3 MB.

Cost. Creating the parser per parse doesn't show in parse time: instructions to parse the same 8 pages, median of 5 interleaved runs, master → this PR: Ruby 2.7.8 103.0 M → 103.3 M, 3.3.10 103.6 → 103.8 M, 3.3.10 YJIT 103.7 → 104.1 M, 4.0.7 102.5 → 102.7 M; each pair sits inside the other arm's min-max range.

Output. SerpApi's Google parser over the 2,760 searches its specs parse, JSON compared byte for byte: master 0375a7b vs 2dff075 and vs fcf1a89, 0 differ each (Ruby 2.7.8).

Not covered: process RSS was not measured (the sandbox used here blocks ps); jemalloc numbers only. Windows and precompiled gems not built; Ruby 3.1, 3.2 not run.

Checks on fcf1a89 (macOS arm64, base master 0375a7b)

Clean git archive, gem build, gem install, specs against the installed gem with lib/ off the load path (CI's test:gem); the new spec parses on 16 threads with GC.start between parses and checks every document:

Ruby 2.7.8   313 runs, 4661 assertions, 0 failures, 0 errors, 0 skips
Ruby 3.3.10  313 runs, 4648 assertions, 0 failures, 5 errors   (nokogiri not installed; same 5 on master)
Ruby 3.4.8   313 runs, 4661 assertions, 0 failures, 0 errors, 0 skips
Ruby 4.0.7   313 runs, 4648 assertions, 0 failures, 5 errors   (same)

ASAN + UBSAN on 2dff075 (Debian 12 arm64, gcc 12, Ruby 3.1, lexbor and the extension instrumented):

spec suite        313 runs, 0 failures, 0 ASAN/UBSAN reports
16-thread stress  96 parses of 8 Google pages, 7,680 css/at_css calls, DOM writes, GC.start, GC.stress spots: 0 errors, 0 reports

fcf1a89 only removes the now-dead if (html_parser == NULL) wrapper; same calls in the same order.

CI note: the Windows install-gem-and-test jobs fail on master 0375a7b too (cannot load such file -- nokolexbor, CI run of 2026-09-27), so they are not caused by this change. Changed C lines pass the repository's .clang-format (git clang-format --diff origin/master, clang-format 23.1.3).

ilyazub and others added 2 commits October 5, 2026 05:19
Create a fresh HTML parser per document parse, destroy it after Lexbor returns the document, and remove the now-unused HTML parser pthread key/destructor. Keep CSS parser and selector thread-local storage unchanged.

Validation:
- ruby -Ilib -Ispec -e 'Dir["spec/**/*_spec.rb"].each { |f| require File.expand_path(f) }': 313 runs, 4661 assertions, 0 failures, 0 errors, 0 skips
- ASAN suite: 313 runs, 4661 assertions, 0 failures, 0 errors, 0 skips
- ASAN 16-thread parse/css/inner_html stress: 384 parses/mutations, 0 failures
- 2,760-search SerpApi gate vs origin/master local build: 0 differ
- 10,000 parses on 16 threads after GC: RSS 193.8 -> 172.1 MiB, jemalloc allocated 108.8 -> 78.1 MiB

This is by GitHub Copilot.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
With the parser no longer cached per thread, `html_parser` is always NULL
on entry, so the `if (html_parser == NULL)` wrapper and the reset before
raising were dead code. Same calls in the same order; no behavior change.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant