WIP: perf(host): three per-call costs on hot paths removed (own-thread lookups, a Signal with no waiter, switches read once) #156

Draft
klimovich008 wants to merge 9 commits from klimovich008/Longshot:perf/os-small-calls into main
Contributor

Ported from Hookshot: https://github.com/NotRexed/Hookshot/pull/2708 (same commits, rebased onto Longshot main with the Longshot names).

Reviews and runs there:

Why

The game calls a few nn::os and host paths thousands of times a frame, and each call does work whose answer is already known. This PR removes that work and keeps its result, to save CPU time on the game's threads.

What changed

  • The calling thread's own ThreadType (host/hos_thread.c thread_arg): GetThreadPriority, GetThreadId, ChangeThreadPriority and the others scanned every thread under threads_lock. The game mostly asks about its own thread (GetThreadPriority(GetCurrentThread()), about 2,400 a frame in Hyrule Field).
    • When the ThreadType is the caller's own, the caller's record answers without the scan.
    • Any other ThreadType is looked up as before. The result is the same, so there is no switch.
  • SignalConditionVariable with no waiter (host/nn_os_sync.c): it returns at once when the condition variable's waiter count is 0, without the lock pair and the wake call. HOS_CV_SIGNAL_ALWAYS=1 gives the old Signal.
    • Ordering: a waiter counts itself in (cv_count_in, a seq-cst add) before it takes the condition variable's lock, and so before it gives the guest mutex back (mutex_release_all). A signaller that holds the mutex therefore sees the waiter, and the waiter cannot be skipped. A signal that reads 0 was made before the waiter started to wait. The bookkeeping under the lock (mixed mutexes, hand-off reservations) uses its own count, cv_in, as before.
    • A signaller that does not hold the mutex could already miss a waiter that had not yet started to wait, before this change as well. That is the condition-variable contract.
  • Switches read once (gpu/vk/vk_menu.cpp, host/nn_fs.c): menu_vsync (RX_VK_FIFO / RX_VK_IMMEDIATE, twice a present) and HOS_FS_OVERLAY (every OpenFile) were read from the environment on every call. They hold for a whole run. Each is now read once and published atomically (HOS_FS_OVERLAY as one atomic pointer).
    • RX_VK_DUMP_STAGES stays a per-read getenv: vk_shader.cpp is part of the program cache's stamp, so changing it would make every player's prepared shaders stale.
    • RX_VK_TAAU_KEYS stays a per-dispatch read: tests/test_taau_env_lookup.py pins that.
  • docs/development.md: HOS_CV_SIGNAL_ALWAYS in the switch reference.

Flags and settings

Flag What it does Default now Recommendation Helps on Doesn't help on Untested on
HOS_CV_SIGNAL_ALWAYS (set) SignalConditionVariable takes its lock and wakes even with no waiter, as before this PR off (the early return is on) off by default; a control for testing a hang or a movie stutter against the old Signal — — Linux in this form (mooki000 ran the earlier head)

RX_VK_FIFO, RX_VK_IMMEDIATE and HOS_FS_OVERLAY keep their meaning; they are now read once a run.

Measurements

Measured on Hookshot, not yet on Longshot. Longshot's code in these files is the same as Hookshot's at the commits measured.

main: 46780503 (2026-11-12, Hookshot, with #2602)
this PR: Hookshot measure/os46 @ 3ca904eb: this PR's changes together with Hookshot #2709 (the sleep and spin changes, since merged there), over 46780503
Baseline: main @ 46780503 (Hookshot). Not Longshot main against this PR: these runs were made on Hookshot before the move, and this PR was not measured alone on that test.
Hardware: RX 9070 XT, AMD driver 26.10.2, Ryzen 7 5700X3D (16 logical processors), Windows 11
Scene: the standard Hyrule Field test as it was then: right stick held right from the load, HOS_FPS_MAX=240, 2560x720 (32:9), 20 s window, warm caches
Method: 6 alternating pairs a cell, medians; 8 and 6 threads: the game held to 0xFF / 0x3F
How to reproduce: claude-perf/std_test.py (the team's bench) with the hyrule scene; not a repo script

main @ 46780503 (Hookshot) this PR @ 3ca904eb, with Hookshot #2709 (Hookshot)
16 threads: fps 52.85 52.7
16 threads: CPU ms a frame 77.3 72.9 (−5.8 %)
16 threads: p99 / worst ms 25.7 / 439.1 24.7 / 87.9
16 threads: runs with a frame over 100 / 250 ms 1 / 1 0 / 0
8 threads: fps 44.05 44.05
8 threads: CPU ms a frame 87.1 80.1 (−8.0 %)
8 threads: p99 / worst ms 29.4 / 88.9 29.0 / 60.5
8 threads: runs with a frame over 100 / 250 ms 0 / 0 0 / 0
6 threads: fps 41.35 40.7 (−1.6 %)
6 threads: CPU ms a frame 80.93 80.91
6 threads: p99 / worst ms 33.0 / 136.5 32.1 / 117.5
6 threads: runs with a frame over 100 / 250 ms 1 / 0 1 / 0
  • The 6-thread cell was measured with #2709's 5 µs spin, which #2709 then limited to 7+ logical processors, so it does not apply to this PR alone.
  • This PR's own share is not separated in these runs.
  • A counter run on 46780503 (the standard test, n=1) counted ~2,800 SignalConditionVariable calls a frame in the play window, none of them with a waiter: the case the early return skips.
  • This PR alone (Brett-D, on Hookshot): RTX 5090 (621.14), i7-13700K, Windows 11, Kokiri Forest moving route, 3 paired rounds, f5a854f3 against 46780503. fps −0.27 (sd 0.68) at 2048x1152 and +0.58 (sd 1.10) with DLSS + frame generation at 165, both within noise; the frame there is limited by the replay on the Presentation thread. CPU per thread at 2048x1152: ModuleSystemWorker1-5 170 → 161 % of a core, all threads 574 → 562 %.
  • Signal switch on Linux (mooki000, on Hookshot's earlier head): RTX 4090, Arch Linux. No lasting hang in either mode. 1,909 early returns a frame in the forest and 634 in the field entrance corridor; no CPU comparison under that load.
  • The story movie's skipped pictures: mooki000 saw 174 with the old Signal and 1,740 with the new in one shared pair. The same check on Windows (Hookshot 46780503 + this PR, the prologue movie, 3 alternating pairs on one build): 530 / 346 / 356 with HOS_CV_SIGNAL_ALWAYS=1, 454 / 337 / 379 without. All six played to the end.
(numbers from the team's std_test summaries and the PR comments linked above; no Longshot run yet)

Testing

Tested: this branch on Longshot main 39647e1a (then rebased onto 1be180bb, which adds only release-script changes), Windows 11, Ryzen 7 5700X3D:

  • tests/test_thread_arg_self.py (new): thread_arg from the source, over own and other ThreadTypes. Broken variants must fail: every call scanning, and the caller's record returned for any ThreadType.
  • tests/test_cv_signal_early.py (new): Signal over waiter counts, with and without HOS_CV_SIGNAL_ALWAYS. Broken variants must fail: the early return taken even with the switch, the early return reading the wrong count, a waiter that does not count itself in. Structural checks: in both waits cv_count_in comes before the condition variable's lock, and cv_enter before mutex_release_all.
  • tests/test_cv_signal_race.py (new): the real Signal, Wait and TimedWait on threads (pthreads for the Win32 calls).
    • A waiter is held between its generation read and cv_enter while a thread that does not own the mutex signals. It must wake: 20 rounds each wait, with and without HOS_CV_SIGNAL_ALWAYS=1.
    • 20,000 hand-offs signalled under the mutex must never stall.
    • It fails on Hookshot's earlier head that counted the waiter under the lock, and on a broken variant doing the same.
  • tests/test_vsync_row.py: one process per switch setting now (the value is read once), with a negative control.
  • python -I -B tests/run_all.py (LONGSHOT_MINGW = llvm-mingw 20261104): 468 passed, 69 skipped, 4 failed. The 4 fail on main on this PC too: nx2/test_nup_paths.py, tests/test_check_third_party.py, tests/test_setup_runtime.py, tests/test_release.py.
  • Syntax compile of every host/*.c (llvm-mingw clang, against a build's generated runtime headers): the changed files compile. 5 files stop at library headers this PC lacks (dav1d, zstd, opus, libavcodec), the same on main.
  • On Hookshot: the in-game runs above, and a gpt-6.1-sol review (three rounds, the last clean).

Not tested: this PR alone on the standard test; a Longshot build in the game; other scenes; Linux with the Signal fix (f5a854f3); macOS.

Needs other testers:

  • Linux (any GPU): the story movie after the Deku Tree's blue warp, HOS_CV_SIGNAL_ALWAYS=1 against unset on one build, counting skipped pictures, expecting no consistent gap between the two (request to open)

Screenshots

Nothing on screen.

Risk and rollback

A missed wake would hang a thread waiting on a condition variable. The ordering above and tests/test_cv_signal_race.py cover it; HOS_CV_SIGNAL_ALWAYS=1 turns the early return off. The own-thread lookup and the switch reads give the same results as before; reverting the commits is enough.

Reviewer notes

Merge order with #157 (BroadcastConditionVariable): both change host/nn_os_sync.c next to Signal and merge without a textual conflict. Whichever lands second needs a short follow-up:

  • the Broadcast reserves the mutex for cv_in - cv_owed, the waiters under the condition variable's lock, instead of cv_waiting;
  • that PR's test fixture gains cv_in;
  • a new test case: a waiter counted in but not yet under the lock must not be owed the mutex.

The follow-up is ready, applies to both orders on Longshot, and will be pushed right after the first of the two merges.

Issues

None.

Checklist

For everyone

  • One topic.
  • The title is a Conventional Commit (type(scope): description).
  • Rebased on current main.
  • python -I -B tests/run_all.py run and passing (any failure that also fails on main is named above).
  • Tested in the game, or what wasn't is named under Testing.
  • Testing filled in: what was tested, what wasn't, and what needs other testers (with test requests).
  • Measured: main and this PR at named commits, on the same PC, scene and settings, at least 2 interleaved runs each, the median reported. (Measured on Hookshot 46780503 only; a Longshot measurement of this PR alone is still to do, so this stays WIP.)
  • No game files, dumps, build output or generated files.
  • third_party/ untouched (a new folder with a PROVENANCE.md instead).
  • The license is OK: GPL-3.0-or-later, nothing incompatible and nothing decompiled.

If applicable

  • AI assistance recorded with Assisted-by: / Co-Authored-By: trailers.
  • Flags noted, and the switch reference updated (docs/development.md, the README).
  • A game-free test added for the fix, failing on main and passing here.
  • A check under tests/ that reads changed source updated in this PR, not weakened. (tests/test_vsync_row.py: one process per switch setting, with a negative control.)
  • Host code builds on both Windows (llvm-mingw) and Linux, with UTF-8 paths. (Windows syntax compile here; Linux is left to CI.)
  • Docs updated.

🤖 Generated with Claude Code

Ported from Hookshot: https://github.com/NotRexed/Hookshot/pull/2708 (same commits, rebased onto Longshot main with the Longshot names). Reviews and runs there: - Joey-Magnus approved it at dd57c2b1, before the Signal fix below (f5a854f3 there): https://github.com/NotRexed/Hookshot/pull/2708#pullrequestreview-5876236516 - mooki000's Linux run (RTX 4090, Arch Linux): https://github.com/NotRexed/Hookshot/pull/2708#issuecomment-6833020156 - Brett-D's frame-time check of f5a854f3 (RTX 5090, i7-13700K, Windows 11): https://github.com/NotRexed/Hookshot/pull/2708#issuecomment-6840275519 ## Why The game calls a few nn::os and host paths thousands of times a frame, and each call does work whose answer is already known. This PR removes that work and keeps its result, to save CPU time on the game's threads. ## What changed - **The calling thread's own ThreadType** (`host/hos_thread.c` `thread_arg`): GetThreadPriority, GetThreadId, ChangeThreadPriority and the others scanned every thread under `threads_lock`. The game mostly asks about its own thread (`GetThreadPriority(GetCurrentThread())`, about 2,400 a frame in Hyrule Field). - When the ThreadType is the caller's own, the caller's record answers without the scan. - Any other ThreadType is looked up as before. The result is the same, so there is no switch. - **SignalConditionVariable with no waiter** (`host/nn_os_sync.c`): it returns at once when the condition variable's waiter count is 0, without the lock pair and the wake call. `HOS_CV_SIGNAL_ALWAYS=1` gives the old Signal. - **Ordering:** a waiter counts itself in (`cv_count_in`, a seq-cst add) before it takes the condition variable's lock, and so before it gives the guest mutex back (`mutex_release_all`). A signaller that holds the mutex therefore sees the waiter, and the waiter cannot be skipped. A signal that reads 0 was made before the waiter started to wait. The bookkeeping under the lock (mixed mutexes, hand-off reservations) uses its own count, `cv_in`, as before. - A signaller that does not hold the mutex could already miss a waiter that had not yet started to wait, before this change as well. That is the condition-variable contract. - **Switches read once** (`gpu/vk/vk_menu.cpp`, `host/nn_fs.c`): `menu_vsync` (`RX_VK_FIFO` / `RX_VK_IMMEDIATE`, twice a present) and `HOS_FS_OVERLAY` (every OpenFile) were read from the environment on every call. They hold for a whole run. Each is now read once and published atomically (`HOS_FS_OVERLAY` as one atomic pointer). - `RX_VK_DUMP_STAGES` stays a per-read `getenv`: `vk_shader.cpp` is part of the program cache's stamp, so changing it would make every player's prepared shaders stale. - `RX_VK_TAAU_KEYS` stays a per-dispatch read: `tests/test_taau_env_lookup.py` pins that. - `docs/development.md`: `HOS_CV_SIGNAL_ALWAYS` in the switch reference. ## Flags and settings | Flag | What it does | Default now | Recommendation | Helps on | Doesn't help on | Untested on | |---|---|---|---|---|---|---| | `HOS_CV_SIGNAL_ALWAYS` (set) | SignalConditionVariable takes its lock and wakes even with no waiter, as before this PR | off (the early return is on) | off by default; a control for testing a hang or a movie stutter against the old Signal | — | — | Linux in this form (mooki000 ran the earlier head) | `RX_VK_FIFO`, `RX_VK_IMMEDIATE` and `HOS_FS_OVERLAY` keep their meaning; they are now read once a run. ## Measurements Measured on Hookshot, not yet on Longshot. Longshot's code in these files is the same as Hookshot's at the commits measured. main: 46780503 (2026-11-12, Hookshot, with #2602) this PR: Hookshot measure/os46 @ 3ca904eb: this PR's changes together with Hookshot #2709 (the sleep and spin changes, since merged there), over 46780503 Baseline: main @ 46780503 (Hookshot). Not Longshot main against this PR: these runs were made on Hookshot before the move, and this PR was not measured alone on that test. Hardware: RX 9070 XT, AMD driver 26.10.2, Ryzen 7 5700X3D (16 logical processors), Windows 11 Scene: the standard Hyrule Field test as it was then: right stick held right from the load, `HOS_FPS_MAX=240`, 2560x720 (32:9), 20 s window, warm caches Method: 6 alternating pairs a cell, medians; 8 and 6 threads: the game held to `0xFF` / `0x3F` How to reproduce: `claude-perf/std_test.py` (the team's bench) with the hyrule scene; not a repo script | | main @ 46780503 (Hookshot) | this PR @ 3ca904eb, with Hookshot #2709 (Hookshot) | |---|---|---| | 16 threads: fps | 52.85 | 52.7 | | 16 threads: CPU ms a frame | 77.3 | 72.9 (−5.8 %) | | 16 threads: p99 / worst ms | 25.7 / 439.1 | 24.7 / 87.9 | | 16 threads: runs with a frame over 100 / 250 ms | 1 / 1 | 0 / 0 | | 8 threads: fps | 44.05 | 44.05 | | 8 threads: CPU ms a frame | 87.1 | 80.1 (−8.0 %) | | 8 threads: p99 / worst ms | 29.4 / 88.9 | 29.0 / 60.5 | | 8 threads: runs with a frame over 100 / 250 ms | 0 / 0 | 0 / 0 | | 6 threads: fps | 41.35 | 40.7 (−1.6 %) | | 6 threads: CPU ms a frame | 80.93 | 80.91 | | 6 threads: p99 / worst ms | 33.0 / 136.5 | 32.1 / 117.5 | | 6 threads: runs with a frame over 100 / 250 ms | 1 / 0 | 1 / 0 | - The 6-thread cell was measured with #2709's 5 µs spin, which #2709 then limited to 7+ logical processors, so it does not apply to this PR alone. - This PR's own share is not separated in these runs. - A counter run on 46780503 (the standard test, n=1) counted ~2,800 SignalConditionVariable calls a frame in the play window, none of them with a waiter: the case the early return skips. - **This PR alone (Brett-D, on Hookshot):** RTX 5090 (621.14), i7-13700K, Windows 11, Kokiri Forest moving route, 3 paired rounds, f5a854f3 against 46780503. fps −0.27 (sd 0.68) at 2048x1152 and +0.58 (sd 1.10) with DLSS + frame generation at 165, both within noise; the frame there is limited by the replay on the Presentation thread. CPU per thread at 2048x1152: ModuleSystemWorker1-5 170 → 161 % of a core, all threads 574 → 562 %. - **Signal switch on Linux (mooki000, on Hookshot's earlier head):** RTX 4090, Arch Linux. No lasting hang in either mode. 1,909 early returns a frame in the forest and 634 in the field entrance corridor; no CPU comparison under that load. - **The story movie's skipped pictures:** mooki000 saw 174 with the old Signal and 1,740 with the new in one shared pair. The same check on Windows (Hookshot 46780503 + this PR, the prologue movie, 3 alternating pairs on one build): 530 / 346 / 356 with `HOS_CV_SIGNAL_ALWAYS=1`, 454 / 337 / 379 without. All six played to the end. ``` (numbers from the team's std_test summaries and the PR comments linked above; no Longshot run yet) ``` ## Testing **Tested:** this branch on Longshot main 39647e1a (then rebased onto 1be180bb, which adds only release-script changes), Windows 11, Ryzen 7 5700X3D: - `tests/test_thread_arg_self.py` (new): `thread_arg` from the source, over own and other ThreadTypes. Broken variants must fail: every call scanning, and the caller's record returned for any ThreadType. - `tests/test_cv_signal_early.py` (new): Signal over waiter counts, with and without `HOS_CV_SIGNAL_ALWAYS`. Broken variants must fail: the early return taken even with the switch, the early return reading the wrong count, a waiter that does not count itself in. Structural checks: in both waits `cv_count_in` comes before the condition variable's lock, and `cv_enter` before `mutex_release_all`. - `tests/test_cv_signal_race.py` (new): the real Signal, Wait and TimedWait on threads (pthreads for the Win32 calls). - A waiter is held between its generation read and `cv_enter` while a thread that does not own the mutex signals. It must wake: 20 rounds each wait, with and without `HOS_CV_SIGNAL_ALWAYS=1`. - 20,000 hand-offs signalled under the mutex must never stall. - It fails on Hookshot's earlier head that counted the waiter under the lock, and on a broken variant doing the same. - `tests/test_vsync_row.py`: one process per switch setting now (the value is read once), with a negative control. - `python -I -B tests/run_all.py` (`LONGSHOT_MINGW` = llvm-mingw 20261104): 468 passed, 69 skipped, 4 failed. The 4 fail on main on this PC too: `nx2/test_nup_paths.py`, `tests/test_check_third_party.py`, `tests/test_setup_runtime.py`, `tests/test_release.py`. - Syntax compile of every `host/*.c` (llvm-mingw clang, against a build's generated runtime headers): the changed files compile. 5 files stop at library headers this PC lacks (dav1d, zstd, opus, libavcodec), the same on main. - On Hookshot: the in-game runs above, and a gpt-6.1-sol review (three rounds, the last clean). **Not tested:** this PR alone on the standard test; a Longshot build in the game; other scenes; Linux with the Signal fix (f5a854f3); macOS. **Needs other testers:** - Linux (any GPU): the story movie after the Deku Tree's blue warp, `HOS_CV_SIGNAL_ALWAYS=1` against unset on one build, counting skipped pictures, expecting no consistent gap between the two (request to open) ## Screenshots Nothing on screen. ## Risk and rollback A missed wake would hang a thread waiting on a condition variable. The ordering above and `tests/test_cv_signal_race.py` cover it; `HOS_CV_SIGNAL_ALWAYS=1` turns the early return off. The own-thread lookup and the switch reads give the same results as before; reverting the commits is enough. ## Reviewer notes **Merge order with #157 (BroadcastConditionVariable):** both change `host/nn_os_sync.c` next to Signal and merge without a textual conflict. Whichever lands second needs a short follow-up: - the Broadcast reserves the mutex for `cv_in - cv_owed`, the waiters under the condition variable's lock, instead of `cv_waiting`; - that PR's test fixture gains `cv_in`; - a new test case: a waiter counted in but not yet under the lock must not be owed the mutex. The follow-up is ready, applies to both orders on Longshot, and will be pushed right after the first of the two merges. ## Issues None. ## Checklist ### For everyone - [x] One topic. - [x] The title is a Conventional Commit (`type(scope): description`). - [x] Rebased on current `main`. - [x] `python -I -B tests/run_all.py` run and passing (any failure that also fails on `main` is named above). - [x] Tested in the game, or what wasn't is named under Testing. - [x] Testing filled in: what was tested, what wasn't, and what needs other testers (with test requests). - [ ] Measured: main and this PR at named commits, on the same PC, scene and settings, at least 2 interleaved runs each, the median reported. (Measured on Hookshot 46780503 only; a Longshot measurement of this PR alone is still to do, so this stays WIP.) - [x] No game files, dumps, build output or generated files. - [x] `third_party/` untouched (a new folder with a `PROVENANCE.md` instead). - [x] The license is OK: GPL-3.0-or-later, nothing incompatible and nothing decompiled. ### If applicable - [x] AI assistance recorded with `Assisted-by:` / `Co-Authored-By:` trailers. - [x] Flags noted, and the switch reference updated (docs/development.md, the README). - [x] A game-free test added for the fix, failing on `main` and passing here. - [x] A check under `tests/` that reads changed source updated in this PR, not weakened. (`tests/test_vsync_row.py`: one process per switch setting, with a negative control.) - [ ] Host code builds on both Windows (llvm-mingw) and Linux, with UTF-8 paths. (Windows syntax compile here; Linux is left to CI.) - [x] Docs updated. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
thread_arg (GetThreadPriority, GetThreadId, ChangeThreadPriority...) scanned every thread under threads_lock; the game
asks about its own thread most of the time (GetThreadPriority(GetCurrentThread()), ~2,400 a frame in Hyrule). The
calling thread's record answers that, compared with its current ThreadType.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
The waiter count is read atomically before the lock pair and wake call; waiters count themselves in under the
condition variable's lock before they give the guest mutex back, so a signaller holding the mutex sees them.
HOS_CV_SIGNAL_ALWAYS=1: as before.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
menu_vsync (two getenv a present), HOS_FS_OVERLAY (every OpenFile) and RX_VK_DUMP_STAGES (every code read) asked the
environment each time. They are set for a whole run; the V-Sync fixture now runs a process per switch setting.
(RX_VK_TAAU_KEYS stays per dispatch: tests/test_taau_env_lookup.py pins that read, about once a frame.)

Co-Authored-By: Claude Opus 5.5 <[email protected]>
(sol review: two first OpenFile calls could see the flag set before the pointer.)

Co-Authored-By: Claude Opus 5.5 <[email protected]>
(sol review round 2: the first signals of several threads read and wrote it at once.)

Co-Authored-By: Claude Opus 5.5 <[email protected]>
SignalConditionVariable returns at once when cv_waiting reads 0. A waiter counted itself in inside cv_enter, under
cv->lk, after its generation read. A signal made without the guest mutex could read 0 in between and return, where the
old Signal waited for cv->lk and woke that waiter. The waits now count the waiter in (cv_count_in) before they take
cv->lk, so a signal that reads 0 came before the waiter started to wait: the case the condition-variable contract lets
any implementation miss. The bookkeeping under cv->lk (mixed mutexes, reservations) moves to cv_in, with main's
semantics unchanged.

tests/test_cv_signal_race.py (new) runs the real Signal, Wait and TimedWait on threads (pthreads for the Win32 calls):
a waiter held in that window while a non-owner signals must wake (20 rounds each wait, with and without
HOS_CV_SIGNAL_ALWAYS=1), and 20,000 hand-offs signalled under the mutex must never stall. It fails on the previous head
and on a variant counting the waiter under cv->lk. test_cv_signal_early.py follows the count into cv_count_in.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
Co-Authored-By: Claude Opus 5.5 <[email protected]>
docs: HOS_CV_SIGNAL_ALWAYS in the switch reference
Some checks failed
PR description / Description follows the template (pull_request_target) Successful in 5s
Website / Website on Cloudflare (pull_request_target) Has been skipped
CI / Conventional Commit title (pull_request) Has been cancelled
CI / Python source checks (ubuntu-24.04, part 1/2) (pull_request) Has been cancelled
CI / Python source checks (ubuntu-24.04, part 2/2) (pull_request) Has been cancelled
CI / SPIR-V fixtures (global narrow load/store, direct surface atomics, F2I immediate) (pull_request) Has been cancelled
CI / Workflow lint (actionlint) (pull_request) Has been cancelled
Third-party parts / As their notes say (pull_request) Has been cancelled
Third-party parts / The same as upstream (pull_request) Has been cancelled
7da97528a6
Co-Authored-By: Claude Opus 5.5 <[email protected]>
HumanBean added this to the v3.0.0 milestone 2026-11-13 17:49:01 +00:00
HumanBean left a comment

Read-only review of the PR at 7da97528 against main@origin fbd42024. I read the code and did not run any tests.

What's good

  • Own-thread lookup (thread_arg). This is safe. cur is the calling thread's own _Thread_local record. It is set in thread_main and hos_boot_main_thread and is never freed while that thread runs, so a ThreadType or handle being reused elsewhere can't make it stale. When the caller's own guest_tt matches, the old scan would have returned the same record. Threads the host didn't create have cur == NULL, and the cur && guard sends them to the scan as before. hos_thread_adopt_guest updates cur->guest_tt in place, so the main thread's adopted ThreadType still matches. No new TLS is added: cur was already _Thread_local in the exe, not in the renderer DLL.
  • Signal with no waiter. I couldn't find a lost wakeup.
    • cv_count_in is a seq-cst add, made before the waiter takes cv->lk and so before it reads gen.
    • A signaller that holds the guest mutex took it after mutex_release_all's seq-cst exchange, so it always sees the count.
    • A signaller that reads 0 is ordered before the waiter's count-in, and so before its gen read. The old Signal could have run fully at that point with the same result, so the skip loses nothing.
    • Every cv_count_in (Wait and TimedWait) is matched by the __atomic_sub in cv_leave on every path, timeout included.
    • The handoff bookkeeping stays on cv_in under the lock.
    • The later upstream Signal fix is in this port as 61fb0a48 (waiter counted before cv->lk). I couldn't open the Hookshot PR to diff f5a854f3 byte for byte, but the content matches its description.
    • Events, including auto-clear versus manual-clear, are not touched: event_signal still sets signaled unconditionally.
  • Switches read once. Nothing changes these at runtime:
    • longshot_env.txt is applied at boot, before the game or the first present (load_longshot_env).
    • The menu's env_release/env_holds only track rows and never write the environment.
    • The live switches in #36 use their own live_on table, and none of them is RX_VK_FIFO, RX_VK_IMMEDIATE, HOS_FS_OVERLAY or HOS_CV_SIGNAL_ALWAYS.
    • Initialisation is thread-safe: menu_vsync uses a C++ magic static (no -fno-threadsafe-statics anywhere), and the C ones are atomic loads and stores of an idempotent value.
  • Portability. __atomic_* and _Thread_local work on llvm-mingw and on Linux clang/gcc. Neither is new to these files.

Must change

  1. "Rebased on current main" is ticked but false. The PR sits on 1be180bb, and main@origin is 38 commits ahead, including changes to host/hos_thread.c, gpu/vk/vk_menu.cpp and docs/development.md. I see no overlapping hunks, so it should rebase cleanly. Rebase, rerun tests/run_all.py, then tick the box.
  2. This duplicates #209. 209's commit bd2b56d3 ("perf(host): three per-call costs on hot paths removed ... (#2708)") has the same code and the same four tests as this PR, line for line. The only difference is the docs/development.md row, which 209 doesn't have. Agree with 209's author on which PR carries it:
    • If this PR does, 209 drops bd2b56d3.
    • If 209 does, this PR closes, and the docs row and measurements move there.
    • Merging both lands the same change twice, and the second history would be misleading.
  3. The Signal comment describes the code as it was before the fix (nn_os_sync.c:587-594). It says the waiter counts itself in "(cv_enter) under cv->lk" and that a signaller without the mutex "can miss a waiter between its gen snapshot and its sleep". After 61fb0a48 the first statement is wrong and the second can no longer happen. This comment is the safety argument for the riskiest change in the PR, so it should match the code.
  4. The Linux box gives "Linux is left to CI" as its reason, but Forgejo runs no CI (AGENTS.md). Either someone builds the host on Linux with gcc or clang, or ask for a test request. Until then the box needs a true reason beside it.

Should change

  • Merge with #157. The description's plan is right, but the fix has to go in the second PR to merge, not in a follow-up:
    • If 157 lands first, rebasing this PR breaks 157's test, which cuts cv_reserve (now on cv_in) into a fixture Obj without cv_in. CONTRIBUTING expects the check to be updated in the same PR.
    • 157's Broadcast reads cv->cv_waiting with a plain load under cv->lk, while the waits now change it atomically outside that lock. That is a data race, though cv_reserve's cv_in guard keeps the result harmless.
    • Broadcast should count from cv_in, as the description says.
  • Measurements. The numbers are from Hookshot, combined with Hookshot https://github.com/NotRexed/Hookshot/pull/2709. Its own share isn't separated, and the 6-thread cell used a spin #2709 later limited.
    • The only run of this PR alone (Brett-D's, RTX 5090) shows fps within noise and about 2 % less CPU in total.
    • The Measured box is correctly unticked. Before it's ticked, it needs a Longshot A/B of this PR alone: same scene, interleaved, at named commits.
    • The movie skipped-picture gap (174 against 1,740) is only explained by Windows runs that don't reproduce it. The Linux test request it names should be opened before this leaves WIP.
  • Nit (nn_fs.c:383). "set but empty: no overlay, as before" isn't quite right. Before, an empty HOS_FS_OVERLAY (possible on Linux only) made alt "/<path>", a lookup at the filesystem root. The new behaviour is better; the comment should say it changed.
Read-only review of the PR at 7da97528 against main@origin fbd42024. I read the code and did not run any tests. ## What's good - **Own-thread lookup (`thread_arg`).** This is safe. `cur` is the calling thread's own `_Thread_local` record. It is set in `thread_main` and `hos_boot_main_thread` and is never freed while that thread runs, so a ThreadType or handle being reused elsewhere can't make it stale. When the caller's own `guest_tt` matches, the old scan would have returned the same record. Threads the host didn't create have `cur == NULL`, and the `cur &&` guard sends them to the scan as before. `hos_thread_adopt_guest` updates `cur->guest_tt` in place, so the main thread's adopted ThreadType still matches. No new TLS is added: `cur` was already `_Thread_local` in the exe, not in the renderer DLL. - **Signal with no waiter.** I couldn't find a lost wakeup. - `cv_count_in` is a seq-cst add, made before the waiter takes `cv->lk` and so before it reads `gen`. - A signaller that holds the guest mutex took it after `mutex_release_all`'s seq-cst exchange, so it always sees the count. - A signaller that reads 0 is ordered before the waiter's count-in, and so before its `gen` read. The old Signal could have run fully at that point with the same result, so the skip loses nothing. - Every `cv_count_in` (Wait and TimedWait) is matched by the `__atomic_sub` in `cv_leave` on every path, timeout included. - The handoff bookkeeping stays on `cv_in` under the lock. - The later upstream Signal fix is in this port as 61fb0a48 (waiter counted before `cv->lk`). I couldn't open the Hookshot PR to diff f5a854f3 byte for byte, but the content matches its description. - Events, including auto-clear versus manual-clear, are not touched: `event_signal` still sets `signaled` unconditionally. - **Switches read once.** Nothing changes these at runtime: - `longshot_env.txt` is applied at boot, before the game or the first present (`load_longshot_env`). - The menu's `env_release`/`env_holds` only track rows and never write the environment. - The live switches in https://git.magicbean.dev/MagicBeanDev/Longshot/pulls/36 use their own `live_on` table, and none of them is `RX_VK_FIFO`, `RX_VK_IMMEDIATE`, `HOS_FS_OVERLAY` or `HOS_CV_SIGNAL_ALWAYS`. - Initialisation is thread-safe: `menu_vsync` uses a C++ magic static (no `-fno-threadsafe-statics` anywhere), and the C ones are atomic loads and stores of an idempotent value. - **Portability.** `__atomic_*` and `_Thread_local` work on llvm-mingw and on Linux clang/gcc. Neither is new to these files. ## Must change 1. **"Rebased on current main" is ticked but false.** The PR sits on 1be180bb, and main@origin is 38 commits ahead, including changes to `host/hos_thread.c`, `gpu/vk/vk_menu.cpp` and `docs/development.md`. I see no overlapping hunks, so it should rebase cleanly. Rebase, rerun `tests/run_all.py`, then tick the box. 2. **This duplicates https://git.magicbean.dev/MagicBeanDev/Longshot/pulls/209.** 209's commit bd2b56d3 ("perf(host): three per-call costs on hot paths removed ... (#2708)") has the same code and the same four tests as this PR, line for line. The only difference is the `docs/development.md` row, which 209 doesn't have. Agree with 209's author on which PR carries it: - If this PR does, 209 drops bd2b56d3. - If 209 does, this PR closes, and the docs row and measurements move there. - Merging both lands the same change twice, and the second history would be misleading. 3. **The Signal comment describes the code as it was before the fix** (nn_os_sync.c:587-594). It says the waiter counts itself in "(cv_enter) under cv->lk" and that a signaller without the mutex "can miss a waiter between its gen snapshot and its sleep". After 61fb0a48 the first statement is wrong and the second can no longer happen. This comment is the safety argument for the riskiest change in the PR, so it should match the code. 4. **The Linux box gives "Linux is left to CI" as its reason, but Forgejo runs no CI** (AGENTS.md). Either someone builds the host on Linux with gcc or clang, or ask for a test request. Until then the box needs a true reason beside it. ## Should change - **Merge with https://git.magicbean.dev/MagicBeanDev/Longshot/pulls/157.** The description's plan is right, but the fix has to go in the second PR to merge, not in a follow-up: - If 157 lands first, rebasing this PR breaks 157's test, which cuts `cv_reserve` (now on `cv_in`) into a fixture `Obj` without `cv_in`. CONTRIBUTING expects the check to be updated in the same PR. - 157's Broadcast reads `cv->cv_waiting` with a plain load under `cv->lk`, while the waits now change it atomically outside that lock. That is a data race, though `cv_reserve`'s `cv_in` guard keeps the result harmless. - Broadcast should count from `cv_in`, as the description says. - **Measurements.** The numbers are from Hookshot, combined with Hookshot https://github.com/NotRexed/Hookshot/pull/2709. Its own share isn't separated, and the 6-thread cell used a spin #2709 later limited. - The only run of this PR alone (Brett-D's, RTX 5090) shows fps within noise and about 2 % less CPU in total. - The Measured box is correctly unticked. Before it's ticked, it needs a Longshot A/B of this PR alone: same scene, interleaved, at named commits. - The movie skipped-picture gap (174 against 1,740) is only explained by Windows runs that don't reproduce it. The Linux test request it names should be opened before this leaves WIP. - **Nit (nn_fs.c:383).** "set but empty: no overlay, **as before**" isn't quite right. Before, an empty `HOS_FS_OVERLAY` (possible on Linux only) made `alt` `"/<path>"`, a lookup at the filesystem root. The new behaviour is better; the comment should say it changed.
host/nn_fs.c Outdated
@ -378,0 +380,4 @@
const char *ov = __atomic_load_n(&overlay, __ATOMIC_RELAXED);
if (ov && !*ov) {
ov = getenv("HOS_FS_OVERLAY");
if (ov && !*ov) ov = NULL; /* (set but empty: no overlay, as before) */
Owner

Nit: "as before" isn't quite right. Before, an empty HOS_FS_OVERLAY (possible on Linux; Windows _putenv("X=") unsets it) made alt "/<path>", a lookup at the filesystem root. Turning that off is better; the comment should say the behaviour changed.

Nit: "as before" isn't quite right. Before, an empty `HOS_FS_OVERLAY` (possible on Linux; Windows `_putenv("X=")` unsets it) made `alt` `"/<path>"`, a lookup at the filesystem root. Turning that off is better; the comment should say the behaviour changed.
@ -461,3 +469,3 @@
/* a signal (cv->lk held): the mutex reserved for the waiter it wakes */
static void cv_reserve(Obj *cv) {
if (!handoff_on() || cv->cv_mixed || !cv->cv_mutex || cv->cv_waiting <= cv->cv_owed || cv->cv_owed >= 8) return;
if (!handoff_on() || cv->cv_mixed || !cv->cv_mutex || cv->cv_in <= cv->cv_owed || cv->cv_owed >= 8) return;
Owner

With #157 this needs fixing in the PR that merges second, not in a follow-up:

  • 157's Broadcast loops cv->cv_waiting - cv->cv_owed with a plain read under cv->lk, while the waits now change cv_waiting atomically outside that lock. That is a data race, though harmless here because this guard stops at cv_in.
  • 157's fixture Obj has no cv_in, so its test stops compiling once cv_reserve reads it.

Broadcast should count from cv_in, and 157's fixture needs cv_in.

With https://git.magicbean.dev/MagicBeanDev/Longshot/pulls/157 this needs fixing in the PR that merges second, not in a follow-up: - 157's Broadcast loops `cv->cv_waiting - cv->cv_owed` with a plain read under `cv->lk`, while the waits now change `cv_waiting` atomically outside that lock. That is a data race, though harmless here because this guard stops at `cv_in`. - 157's fixture `Obj` has no `cv_in`, so its test stops compiling once `cv_reserve` reads it. Broadcast should count from `cv_in`, and 157's fixture needs `cv_in`.
@ -578,1 +586,4 @@
HLE(_ZN2nn2os25FinalizeConditionVariableEPNS0_21ConditionVariableTypeE) { drop(ARG(0)); }
/* A signal with no waiter returns at once (~2,500 signals a frame in Hyrule: each took cv->lk and made a wake call, a
waiter there or not), as glibc's pthread_cond_signal does, and as Horizon's user mode does when the key reads 0. No
waiter that can still miss its predicate's change is missed: a waiter counts itself in (cv_enter) under cv->lk before
Owner

This comment describes the code as it was before 61fb0a48. The waiter now counts itself in with cv_count_in, before cv->lk, not in cv_enter under it. A signaller that doesn't hold the mutex can no longer miss a waiter between its gen snapshot and its sleep: closing that gap was the point of the fix. Please restate the argument as the code has it now. A signal that reads 0 is ordered before the waiter's count-in, and so before its gen read, so the skipped signal is one the old Signal could also have made before the wait began.

This comment describes the code as it was before 61fb0a48. The waiter now counts itself in with `cv_count_in`, before `cv->lk`, not in `cv_enter` under it. A signaller that doesn't hold the mutex can no longer miss a waiter between its gen snapshot and its sleep: closing that gap was the point of the fix. Please restate the argument as the code has it now. A signal that reads 0 is ordered before the waiter's count-in, and so before its `gen` read, so the skipped signal is one the old Signal could also have made before the wait began.
Owner

I rebased your eight commits onto main @ fbd42024 (no conflicts) and fixed the two comments from my review in one commit on top. It's pushed as 222f568e, and your commits are unchanged apart from the rebase.

Two corrections to my review:

  • I wrote that "Forgejo runs no CI". That was wrong: it does. Your Linux "left to CI" box is fine as it is.
  • Please ignore the point about #209 for now.

My commit, which changes comments only:

  • The Signal comment in host/nn_os_sync.c now describes the code as it is after 61fb0a48:

    • the waiter counts itself in with cv_count_in (sequentially consistent) while it still holds the guest mutex, before it takes cv->lk and reads gen;
    • why a signaller that reads 0 is ordered before the wait, so the early return skips only a signal the old one could have made at that point;
    • every count-in has its count-out in cv_leave on every way out of Wait and TimedWait, a timeout included.

    I checked the order against both waits: count-in, cv->lk, mutex_release_all, then cv_leave.

  • host/nn_fs.c's empty HOS_FS_OVERLAY comment now says this changed: before, an empty value, which only Linux can set, made every file look up at the filesystem root.

No tests were run here. The checks that cut these functions don't match on comment text. Still open from the review: a Longshot A/B (the Measured box), and whichever of this PR and #157 lands second updating the other's fixture.

I rebased your eight commits onto main @ fbd42024 (no conflicts) and fixed the two comments from my review in one commit on top. It's pushed as 222f568e, and your commits are unchanged apart from the rebase. **Two corrections to my review:** - I wrote that "Forgejo runs no CI". That was wrong: it does. Your Linux "left to CI" box is fine as it is. - Please ignore the point about https://git.magicbean.dev/MagicBeanDev/Longshot/pulls/209 for now. **My commit**, which changes comments only: - **The Signal comment** in `host/nn_os_sync.c` now describes the code as it is after 61fb0a48: - the waiter counts itself in with `cv_count_in` (sequentially consistent) while it still holds the guest mutex, before it takes `cv->lk` and reads `gen`; - why a signaller that reads 0 is ordered before the wait, so the early return skips only a signal the old one could have made at that point; - every count-in has its count-out in `cv_leave` on every way out of Wait and TimedWait, a timeout included. I checked the order against both waits: count-in, `cv->lk`, `mutex_release_all`, then `cv_leave`. - **`host/nn_fs.c`'s empty `HOS_FS_OVERLAY` comment** now says this changed: before, an empty value, which only Linux can set, made every file look up at the filesystem root. No tests were run here. The checks that cut these functions don't match on comment text. Still open from the review: a Longshot A/B (the Measured box), and whichever of this PR and https://git.magicbean.dev/MagicBeanDev/Longshot/pulls/157 lands second updating the other's fixture.
HumanBean force-pushed perf/os-small-calls from 7da97528a6
Some checks failed
PR description / Description follows the template (pull_request_target) Successful in 5s
Website / Website on Cloudflare (pull_request_target) Has been skipped
CI / Conventional Commit title (pull_request) Has been cancelled
CI / Python source checks (ubuntu-24.04, part 1/2) (pull_request) Has been cancelled
CI / Python source checks (ubuntu-24.04, part 2/2) (pull_request) Has been cancelled
CI / SPIR-V fixtures (global narrow load/store, direct surface atomics, F2I immediate) (pull_request) Has been cancelled
CI / Workflow lint (actionlint) (pull_request) Has been cancelled
Third-party parts / As their notes say (pull_request) Has been cancelled
Third-party parts / The same as upstream (pull_request) Has been cancelled
to 222f568e66
All checks were successful
CI / Python source checks (ubuntu-24.04, part 1/2) (pull_request) Has been skipped
CI / Python source checks (ubuntu-24.04, part 2/2) (pull_request) Has been skipped
CI / SPIR-V fixtures (global narrow load/store, direct surface atomics, F2I immediate) (pull_request) Has been skipped
Third-party parts / The same as upstream (pull_request) Has been skipped
Website / Website on Cloudflare (pull_request) Has been skipped
CI / Conventional Commit title (pull_request) Successful in 4s
PR description / Description follows the template (pull_request_target) Successful in 5s
CI / Workflow lint (actionlint) (pull_request) Successful in 6s
Third-party parts / As their notes say (pull_request) Successful in 6s
2026-11-13 18:05:41 +00:00
Compare
First-time contributor

Code review only; not built or run.

Edited: the Kokiri profile quoted below drew a 5:4 picture, 1800×1440, not 2048×1152 as I first wrote: the PC's display mode had changed that hour. At the usual 16:9 picture the same route runs at about 74 fps. A rerun of the profile there is queued, and I'll update the numbers if they move.

Thanks for porting this over. HumanBean's review covers the ordering argument and the rest, and I found nothing new on correctness. What I checked on my side:

  • cv_count_in and cv_leave pair up on the one way out of WaitConditionVariable and TimedWaitConditionVariable, the timeout included.
  • A live cur never has a zero guest_tt (the main thread's comes from hos_sys_alloc), so the own-thread test can't match a null handle.
  • The environment is only written at start-up (the env file and arguments, the relaunch, SDL's init), so caching getenv's pointer is safe.

What I can add is where these three costs sit in Longshot. This is from a sampled profile of Kokiri Forest on Longshot 1ef93e79 (i7-13700K, RTX 5090, Windows, 1800×1440 (5:4), uncapped, about 79 fps; all threads, named with the build's linker maps):

cost % of a core ms a frame on sead::MainThread and the Presentation Thread
GetThreadPriority under the shared lock 0.7 0.08 0.0
SignalConditionVariable with nobody waiting 0.6 0.07 0.0
getenv on hot paths (menu_vsync and the others) 0.0 0.00 0.0

All of it is on the job workers. In that scene the frame is paced by the main thread's hand-offs to its jobs: the main thread waits for them 51 % of its time, about 6.4 ms a frame. Those hand-offs are light events and light message queues (WaitLightEvent/SignalLightEvent, Send/ReceiveLightMessageQueue), which this PR doesn't touch. So the only way it can shorten that wait is through the ~0.15 ms a frame of worker CPU it frees. On a 24-thread PC I'd expect no change in fps. That fits the Hookshot version of this change (#2708 there), which measured flat on the same PC (−0.27 ± 0.68 fps in Kokiri).

For the Longshot A/B, two suggestions:

  • Report CPU time per thread group as well as fps.
  • A Steam Deck or 6-core run is where the freed CPU is most likely to show in frame times.

No objection from me once HumanBean's points are settled.

Code review only; not built or run. *Edited:* the Kokiri profile quoted below drew a 5:4 picture, 1800×1440, not 2048×1152 as I first wrote: the PC's display mode had changed that hour. At the usual 16:9 picture the same route runs at about 74 fps. A rerun of the profile there is queued, and I'll update the numbers if they move. Thanks for porting this over. HumanBean's review covers the ordering argument and the rest, and I found nothing new on correctness. What I checked on my side: - `cv_count_in` and `cv_leave` pair up on the one way out of `WaitConditionVariable` and `TimedWaitConditionVariable`, the timeout included. - A live `cur` never has a zero `guest_tt` (the main thread's comes from `hos_sys_alloc`), so the own-thread test can't match a null handle. - The environment is only written at start-up (the env file and arguments, the relaunch, SDL's init), so caching `getenv`'s pointer is safe. What I can add is where these three costs sit in Longshot. This is from a sampled profile of Kokiri Forest on Longshot 1ef93e79 (i7-13700K, RTX 5090, Windows, 1800×1440 (5:4), uncapped, about 79 fps; all threads, named with the build's linker maps): | cost | % of a core | ms a frame | on sead::MainThread and the Presentation Thread | |---|---|---|---| | GetThreadPriority under the shared lock | 0.7 | 0.08 | 0.0 | | SignalConditionVariable with nobody waiting | 0.6 | 0.07 | 0.0 | | getenv on hot paths (menu_vsync and the others) | 0.0 | 0.00 | 0.0 | All of it is on the job workers. In that scene the frame is paced by the main thread's hand-offs to its jobs: the main thread waits for them 51 % of its time, about 6.4 ms a frame. Those hand-offs are light events and light message queues (`WaitLightEvent`/`SignalLightEvent`, `Send`/`ReceiveLightMessageQueue`), which this PR doesn't touch. So the only way it can shorten that wait is through the ~0.15 ms a frame of worker CPU it frees. On a 24-thread PC I'd expect no change in fps. That fits the Hookshot version of this change (#2708 there), which measured flat on the same PC (−0.27 ± 0.68 fps in Kokiri). For the Longshot A/B, two suggestions: - Report CPU time per thread group as well as fps. - A Steam Deck or 6-core run is where the freed CPU is most likely to show in frame times. No objection from me once HumanBean's points are settled.
All checks were successful
CI / Python source checks (ubuntu-24.04, part 1/2) (pull_request) Has been skipped
CI / Python source checks (ubuntu-24.04, part 2/2) (pull_request) Has been skipped
CI / SPIR-V fixtures (global narrow load/store, direct surface atomics, F2I immediate) (pull_request) Has been skipped
Third-party parts / The same as upstream (pull_request) Has been skipped
Website / Website on Cloudflare (pull_request) Has been skipped
CI / Conventional Commit title (pull_request) Successful in 4s
PR description / Description follows the template (pull_request_target) Successful in 5s
CI / Workflow lint (actionlint) (pull_request) Successful in 6s
Third-party parts / As their notes say (pull_request) Successful in 6s
This pull request is marked as a work in progress.
This branch is out-of-date with the base branch
View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u 'perf/os-small-calls:klimovich008-perf/os-small-calls'
git switch 'klimovich008-perf/os-small-calls'
Sign in to join this conversation.
No reviewers
No milestone
No project
No assignees
3 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
MagicBeanDev/Longshot!156
No description provided.