Conversation
…path applySignature built a fresh SignatureSolver per video: JSContext setup, meriyah/astring bundle eval, and a full parse of the ~2MB player JS every time — devastating on JIT-less JavaScriptCore (tvOS/iOS), where it costs 15s+ per video. - SignatureSolver.shared(forJS:) caches one solver per player-JS version - First batchSolve requests output_preprocessed and stores the preprocessed player; subsequent solves send it back and skip the parse - Solves serialized with a lock (JSContext is not thread-safe) - Task.checkCancellation before solver work so cancelled extractions don't burn CPU - Timing os_log (.default) around init and solve
Contributor
📝 WalkthroughWalkthrough
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Sources/YouTubeKit/SignatureSolver.swift`:
- Around line 23-37: Update SignatureSolver.shared(forJS:) to compare
cached.playerJS directly with js instead of computing and storing js.hashValue,
eliminating collision-prone caching. While holding sharedLock and before
SignatureSolver initialization, check for task cancellation and abort using the
existing throwing cancellation mechanism.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 235617a3-b1fb-4339-8a86-5189a00b0d60
📒 Files selected for processing (2)
Sources/YouTubeKit/Extraction.swiftSources/YouTubeKit/SignatureSolver.swift
… init Avoids the O(N) hashValue computation and its (rare) collision risk that could return a wrong solver; checks task cancellation inside the lock before the expensive SignatureSolver init.
![codex-pr-bot[bot]](https://avatars.githubusercontent.com/in/4238751?s=60&v=4){kind=small}
The solver's checkCancellation throws CancellationError out of applySignature, which the retry catch treated as a stale-player-JS failure — clearing the shared JS cache and retrying. Propagate cancellation immediately instead, so a cancelled extraction stops cleanly without churning the cache for concurrent extractions.
A single-entry cache means alternating between two player-JS variants in one session (e.g. web vs TV/embed) evicts and rebuilds each time. Keep a small MRU list keyed by player JS so each variant's prepared solver is reused, bounded to cap JSContext/player memory.
Matches the existing pattern for the __js caches; the package supports swift-tools 5.8, where the unconditional attribute fails to parse.
Comment on lines
+253
to
+256
| } catch is CancellationError { | ||
| // Cancellation is not a stale-JS failure — propagate it | ||
| // immediately instead of clearing the cache and retrying. | ||
| throw CancellationError() |
There was a problem hiding this comment.
When methods contains another entry after .local, throwing here does not propagate immediately: the enclosing Task.retry(with: methods) catches every error, including CancellationError, and proceeds to the next extraction method. A cancelled extraction can therefore start the remote WebSocket fallback and potentially return or wait for it instead of terminating promptly.
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
2 participants
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.
Problem
Extraction.applySignatureconstructs a freshSignatureSolverfor every video: a newJSContext, re-evaluation of the meriyah/astring UMD bundles, and a full parse of the ~2 MB player JS on the first solve. On platforms where JavaScriptCore has no JIT (tvOS/iOS), that first parse dominates — it cost ~15 s per video in our measurements on Apple TV, paid again for every title.Since the player JS changes only every few days, this work is almost entirely redundant across videos in a session.
Fix
SignatureSolver.shared(forJS:)caches one instance keyed by the player-JS hash and reuses it across videos.batchSolverequestsoutput_preprocessedand stores the returned preprocessed player; subsequent solves send it back via thepreprocessed_playerinput type and skip the full parse.JSContextisn't thread-safe).Task.checkCancellation()runs before the expensive solver work so a cancelled extraction doesn't burn CPU.Measurements (Apple TV, tvOS)
Verification
Builds clean across the CI matrix locally, including explicit Swift 6 language mode (
nonisolated(unsafe)cache guarded byNSLock).Note
Overview
Reusable signature solving. Reuses
SignatureSolverinstances across videos through a bounded four-entry, most-recently-used cache keyed by the exact player JavaScript, avoiding repeatedJSContextsetup for recurring player versions.Preprocessed players. Stores the preprocessed player returned by the first
batchSolveand uses it for subsequent signature andnchallenge resolution. Solver access is serialized to protect the shared JavaScript context.Cancellation handling. Checks for cancellation around expensive solver work and propagates
CancellationErrordirectly, preventing cancellation from being treated as stale player JavaScript and triggering an unnecessary cache refresh.