Skip to content

fix(scripting/node): run promise continuations after events and ref calls - #4404

Open
OneEarJoe5 wants to merge 1 commit into
citizenfx:masterfrom
OneEarJoe5:fix/node-drain-microtasks
Open

OneEarJoe5 wants to merge 1 commit into
citizenfx:masterfrom
OneEarJoe5:fix/node-drain-microtasks

Conversation

@OneEarJoe5

Copy link
Copy Markdown

Goal of this PR

In server-side JS, every await on something that comes back from another resource takes ~50 ms, one server frame. That's any event from another resource and any callback/export from another resource, e.g. await on exports.oxmysql.query(...) from a JS resource, or a Lua resource calling back into JS. The other resource answers right away, but the promise continuation only runs on our next tick.

Events and ref calls go straight into the runtime (TriggerEvent / CallRef), not through its uv loop. The context's microtask queue is explicit, so the continuations they queue sit there until something drains it, which is the resource tick (process._tickCallback() in timer.js) or the next uv callback, if there is one. I mentioned this one in #4402, that PR doesn't change it.

How is this PR achieving the goal

  • TriggerEvent and CallRef set a flag on the runtime after the call.
  • TickFast, which already runs every millisecond for every node runtime (UvLoopTimer), checks the flag after its uv_run and, if set, opens and closes a node::CallbackScope. That's what node does after its own callbacks: it runs the microtasks and the process.nextTick queue and reports unhandled rejections.
  • It runs outside of any JS, so it never runs continuations in the middle of other code (nested calls, a callback called synchronously from an export and so on), and when a ref call returns a promise the caller has already attached its handler by then, so there are no false "unhandled rejection" warnings.
  • They used to run inside the resource tick, so while the profiler records they now get their own scope, callbacks (<resource>), instead of disappearing from recordings.
  • Without the flag set, nothing changes.

I first tried running them right when the outermost event/ref call returns. That gave false unhandled rejection warnings for async callbacks, because the caller only attaches its handler to the returned promise after CallRef returns. Doing it in TickFast avoids that and costs at most ~1 ms.

This PR applies to the following area(s)

Server, ScRT: JS

Successfully tested on

Platforms: Windows (server), Linux (server, WSL2 Ubuntu 24.04)

Windows: server built from current master (5412ed8), with and without this change. Linux: before = the official 38621 artifact (built from that same commit), after = this branch built with the alpine builder from code/tools/ci/docker-builder. Test resources: one JS resource that answers from its own event loop (like a driver after I/O, timers.setImmediate then emit / calling the callback), one Lua resource that answers from its scheduler, one JS resource that awaits all of it, plus oxmysql 2.14.1 against a local MariaDB on Windows.

Case before after (Windows) after (Linux)
await an event answered by another JS resource ~50 ms 0.7–2.5 ms 1.2–1.4 ms
await a callback passed to another JS resource's export ~50 ms 0.7–2.5 ms 1.2–1.4 ms
await a callback a Lua resource calls from its scheduler (from the call to the continuation) ~50 ms 0.07–1.4 ms 0.06–1.2 ms
Lua event into an async JS handler, entry to first continuation ~50 ms 0.26–0.43 ms 0.09–0.71 ms
oxmysql SELECT 1 from JS ~50 ms 0.4–4.5 ms (no MariaDB there)
a continuation that emits into the same resource again, 1000 hops 54–60 ms 4.9 ms 9.5 ms
30 JS resources awaiting callbacks from another resource for 20 s 199–200 awaits each, ~50 ms avg 398 each, 2–6 ms avg 400 each, 2.4–3.2 ms avg
sync callback from an export: our code after the call runs before the continuation after-call, then same same
order of callback / microtasks / nextTick inside it cb, micro1, micro2, tick same same
async callback that throws unhandled rejection warning, keeps going same warning, no extra one same
async event handler from another resource runs same same
resource stopped (from Lua) right after it got an event, 20 times ok ok ok
profiler record, 5 ms of work in each continuation counted under tick (mttest) counted under callbacks (mttest) (252 ms for 50 × 5 ms) same (247 ms)
net event from a player into an async handler 0.06–0.17 ms (already fast, not affected) same –

CPU cost, process CPU time (process.cpuUsage), 3 runs × 5 rounds each on an otherwise idle machine:

before after
100k local emits 469–641 ms 469–609 ms
20k sync export calls 359–500 ms 359–735 ms (one outlier, otherwise 359–516)
30 resources above, server CPU over 20 s 1.8–1.9 s for 6000 awaits 2.8 s for 12000 awaits (less per await)

Also ran my JS timer test resource from #4402 (timers, errors, natives, profiler, resource restart from outside, heap) and txAdmin 8.1.2 starting and running the server with these resources: unchanged, no heartbeat problems.

The client isn't affected: citizen-scripting-node is server-only.

Not tested: C# resources calling back into JS (same CallRef path, I just didn't write a C# test resource).

Checklist

  • Code compiles and has been tested successfully.
  • Code explains itself well and/or is documented.
  • My commit message explains what the changes do and what they are for.
  • No extra compilation warnings are added by these changes.

…alls

Events and ref calls from other resources call straight into the
runtime, not through its uv loop, so the microtasks and nextTicks they
queue only ran on the next resource tick. Every `await` on an event or
a callback from another resource (e.g. an oxmysql query from JS) took
~50 ms.

Mark the runtime after an event or ref call and run those queued
continuations in TickFast, which already runs every millisecond, inside
a node::CallbackScope like node does after its own callbacks. That
happens outside of any JS, so nothing runs in the middle of other code,
and callers still get to attach their handlers to a returned promise
first. While the profiler records, they get their own "callbacks"
scope for the resource, since they no longer run inside its tick.

Signed-off-by: OneEarJoe5 <212275319+OneEarJoe5@users.noreply.github.com>
@github-actions github-actions Bot added ScRT: JS Issues/PRs related to the JavaScript scripting runtime invalid Requires changes before it's considered valid and can be (re)triaged labels Nov 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

invalid Requires changes before it's considered valid and can be (re)triaged ScRT: JS Issues/PRs related to the JavaScript scripting runtime

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant