Repository navigation
Release the GIL during reader searches and allow skipping snippets - #298
Open
chapmanjacobd wants to merge 2 commits into
Open
chapmanjacobd wants to merge 2 commits into
chapmanjacobd wants to merge 2 commits into
Conversation
Release the GIL around archive opening and native full-text execution and iteration. Allow callers to skip snippet generation without changing the default, retain worker-private ownership guidance, and add concurrency and GIL-progress regressions. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Author
|
suggested ordering: merge/release openzim/libzim#1141, bump LIBZIM_DL_VERSION, and only then merge this PR |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR lets independent reader workers search concurrently in threads and adds an option to skip snippets when they are not needed.
Search.get_results(..., include_snippets=True). The default preserves existing behavior;Falseskips native article reads and parsing for snippets, leavingSearchResult.snippetempty.Suggestions, item-content cache synchronization and the existing creator locking contract are unchanged.
Native dependency and thread-safety scope
Threaded reader use should be paired with openzim/libzim#1141, which fixes the existing search guard adopting mutexes that were never locked. This PR does not update the default libzim 9.8.2 bundle; coordinate a native release/bundle update before recommending threaded deployments.
This does not certify every native library version or arbitrary shared-object usage. Workers should open independent
Archivehandles, rather than only creating multipleSearchers from one shared archive.Validation
Benchmark results
Skipping snippets accounts for most of the overall speedup. Compared with multiprocessing using
include_snippets=False, multithreading adds 22% desktop throughput and 47% on the 4-CPU / 2 GiB profile. From multiprocessing withinclude_snippets=Trueto multithreading withinclude_snippets=False, sampled peak PSS falls 62% and 48%, respectively. The 29–43x total speedup is not a claim about GIL release alone.Multiprocessing:
include_snippets=Truevs.include_snippets=FalseThe
include_snippets=Trueconfiguration uses the multiprocessing approach and generates snippets that the application then discards. The comparison usesinclude_snippets=Falsewhile retaining multiprocessing.include_snippets=Trueinclude_snippets=Trueinclude_snippets=Falseinclude_snippets=Trueinclude_snippets=FalseMultiprocessing vs. multithreading (
include_snippets=False)Both configurations use this reader with
include_snippets=False. Multithreading uses a bounded thread pool and independently opened archive/searcher handles.Method
All configurations use the same rebuilt native dependencies: Python 3.13.12, libzim 9.8.2 with openzim/libzim#1141, Xapian 1.4.31 and ICU 78.3. The controlled baseline rebuilds unchanged python-libzim
57555c2against that stack to avoid a native dependency-version confound.The desktop is an i5-13600KF with 20 logical CPUs and 32 GiB RAM, limited to
CPUQuota=2000%andMemoryMax=8192M. The 4-CPU / 2 GiB profile usesCPUQuota=400%andMemoryMax=2048Mon the same host; it is not Raspberry Pi hardware. Swap is disabled for both. Desktop runs use eight source callers and native workers; 4-CPU / 2 GiB profile runs use four of each.Values are medians of three fresh-service trials per configuration. Each trial warms 144 searches, then repeats the 144-search batch for at least three measured seconds. A batch contains twelve queries against each of three archives (Wikipedia astronomy, Wikipedia medicine and Cooking Stack Exchange), with a sixteen-hit limit. One search is one query against one archive.
Measurement notes
CPU includes the driver and all workers. Peak PSS is sampled across the process tree and includes a retained 512 MiB server/model stand-in. Latencies include backend admission waits but exclude source-executor queueing. OS caches remain warm, and file pages may be charged outside the service cgroup. These are native-search microbenchmarks, not whole-server latency, model/index residency guarantees or Raspberry Pi performance. All 81 matrix, baseline and confirmation trials returned the same path/title digest.