Skip to content

feat: allow custom priority for workers#377

Merged
NathanWalker merged 2 commits into
mainfrom
feat/worker-priority
May 28, 2026
Merged

feat: allow custom priority for workers#377
NathanWalker merged 2 commits into
mainfrom
feat/worker-priority

Conversation

@edusperoni
Copy link
Copy Markdown
Collaborator

@edusperoni edusperoni commented May 27, 2026

Summary by CodeRabbit

  • New Features

    • Workers can now be started with an optional Quality of Service (QoS) setting, allowing finer control over worker thread scheduling and priority.
  • Bug Fixes

    • Improved uncaught-exception handling in workers: exceptions are forwarded reliably to the main thread, duplicate reports are prevented, and worker termination/cleanup is more deterministic.

Review Change Stack

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 27, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 5a715743-7606-41d4-bbb9-23584a1b9164

📥 Commits

Reviewing files that changed from the base of the PR and between 7fc278f and 5009176.

📒 Files selected for processing (1)
  • NativeScript/runtime/Worker.mm

📝 Walkthrough

Walkthrough

Adds QoS support for NativeScript workers: the Worker constructor parses an optional iosPriority option and maps it to a numeric QoS value that is passed into WorkerWrapper::Start(...), which applies the QoS to the scheduled NSBlockOperation.

Changes

Worker Thread Quality of Service Support

Layer / File(s) Summary
Worker QoS API Contract
NativeScript/runtime/DataWrapper.h
WorkerWrapper::Start() signature extended to accept optional int qualityOfService = -1.
Header Reorganization and Formatting
NativeScript/runtime/DataWrapper.h
Header includes reordered; wrapper class declarations reformatted without changing public signatures (aside from the Start API change).
iOS Priority Parsing and QoS Propagation
NativeScript/runtime/Worker.mm
Constructor callback parses optional iosPriority string and maps recognized priority names to numeric qos; computed qos is passed to worker->Start(...).
QoS Scheduling Implementation
NativeScript/runtime/WorkerWrapper.mm
Start() creates an NSBlockOperation for the background looper, sets op.qualityOfService when qualityOfService >= 0, and enqueues via the worker queue.
Error Handling Refinements
NativeScript/runtime/Worker.mm, NativeScript/runtime/WorkerWrapper.mm
Debug-mode jsErrorOccurred dispatch restructured to send uncaught-exception payload asynchronously then clear the flag; uncaught-exception dispatch path adds explicit worker->Terminate() and formatting changes around PassUncaughtExceptionFromWorkerToMain and error object construction.
Namespace Comments and Cleanup
NativeScript/runtime/Worker.mm, NativeScript/runtime/WorkerWrapper.mm
Trailing namespace-closing comments added/adjusted at file ends.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

A rabbit hummed by the dev's soft light,
QoS wakes workers to tune their flight.
🐇 Strings to numbers, priorities unfurled—
Background loops now mind their world.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main functional change: adding support for custom priority configuration for workers through the new qualityOfService parameter.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 `@NativeScript/runtime/Worker.mm`:
- Around line 114-120: The ios QoS mapping in the priority branch (checks for
"userInteractive", "userInitiated", "default", "utility") incorrectly hardcodes
hex literals; change those branches to use the NSQualityOfService constants
(e.g., NSQualityOfServiceUserInteractive, NSQualityOfServiceUserInitiated,
NSQualityOfServiceDefault/NSQualityOfServiceUtility) and modify the "default"
branch so it does not force an explicit QoS value (i.e., leave qos unset or set
to the sentinel used by worker->Start(..., qos) to indicate “do not set QoS”);
update the code around worker->Start(..., qos) to pass that sentinel when
priority == "default" and ensure all other branches use the named
NSQualityOfService* constants instead of hex literals.
🪄 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: 236f41b7-1842-487f-8583-141ad44b2b77

📥 Commits

Reviewing files that changed from the base of the PR and between ecd3598 and 7fc278f.

📒 Files selected for processing (3)
  • NativeScript/runtime/DataWrapper.h
  • NativeScript/runtime/Worker.mm
  • NativeScript/runtime/WorkerWrapper.mm

Comment thread NativeScript/runtime/Worker.mm
@NathanWalker NathanWalker merged commit 8d238ed into main May 28, 2026
9 checks passed
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.

2 participants