mirror/LocalAI
1
0
Fork 0
mirror of https://github.com/mudler/LocalAI.git synced 2026-07-30 18:09:05 -04:00
Code Issues Packages Projects Releases Wiki Activity

backend(audio-cpp): make the B10 test able to fail, and document LaneEntry

Browse Source 
Review of the previous commit found the B10 test could not fail for the reason
it was named. It aged the in-flight run to about 120 ms and then tried two
budgets, 30 ms and 60 ms, both under that age, so both callers took the
fail-fast path. "The two failure modes do not share one message" was comparing
two fail-fast messages that differ only in the budget they print, and the
timeout path was never reached. The second budget is now 400 ms, well over the
run's age, so that caller queues and times out, and a new check asserts which
path each caller took instead of inferring it from inequality. A mutant that
makes the fail-fast path emit the timeout message previously died only on B4 and
B8 checks; it now also dies on B10.

Comment-only changes elsewhere. LaneEntry now says it is not reentrant and does
not detect reentrancy: a second entry on a thread that already holds the lane
surfaces as LaneUnavailable with a positive budget, but parks silently in
unbounded mode, which matters because a handler may hold one across a whole
stream. The immovability note now names the shapes that work, an optional
emplaced in place or a unique_ptr, rather than saying to hold the entry
indirectly without saying how; all three documented forms were compiled before
being written down, which is how the note came to say that an optional of an
immovable type cannot itself be returned.

The header's explanation of why fail-fast exists is reworded. Two clauses traced
back to a specification written after reading the Apache-2.0 upstream header,
and while that was judged de minimis, this unit was rewritten precisely to carry
no upstream expression at all.

The margin table in the report was also wrong about which wall-clock margins are
load-sensitive: there are four, not one, and the tightest is the B3 arrival
check, which is now flagged at the call site. No margin value changed and none
moved across 65 runs.

Assisted-by: Claude:claude-opus-5 [Claude Code]
Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
This commit is contained in:
Ettore Di Giacinto
2026-07-26 12:58:36 +00:00
committed by localai-org-maint-bot
parent 724a995f8a
commit 24f2d2a4c6
2 changed files with 49 additions and 9 deletions
Download Patch File Download Diff File Expand all files Collapse all files

36
backend/cpp/audio-cpp/inference_lane.h
View File

@@ -4,12 +4,13 @@
// owns. Standard library only.
//
// Why a plain mutex is not enough: an audio.cpp session is not reentrant, so
// concurrent gRPC handlers have to take turns. But a GPU call that wedges cannot
// be cancelled from userspace, so an unbounded queue behind one wedged run would
// swallow every gRPC worker thread until the process is useless. A caller
// therefore needs to be able to walk away, and needs to be able to tell "the
// lane is busy with normal work and I ran out of patience" apart from "the run
// in the lane has already outlived the patience I brought".
// concurrent gRPC handlers have to take turns. But once a GPU call stops making
// progress there is nothing a host thread can do to take it back, and an
// unbounded queue behind such a run would absorb the handler threads one by one
// until nothing is left to answer with. A caller therefore needs to be able to
// walk away, and needs to be able to tell "the lane is busy with normal work and
// I ran out of patience" apart from "the run in the lane has already outlived
// the patience I brought".
#include <condition_variable>
#include <cstdint>
@@ -93,6 +94,13 @@ class InferenceLane {
// including a caller that returns from the middle of a long stream. Throws
// LaneUnavailable if the lane could not be taken, in which case there is no
// object and nothing to release.
//
// Not reentrant, and it does not detect reentrancy: a second entry constructed
// while the calling thread already holds the same lane waits for a lane only
// that thread can release. With a positive budget that surfaces as
// LaneUnavailable, but in unbounded mode the thread parks with no diagnostic at
// all. Keep entries one per call: a handler that holds one across a stream must
// not let a helper it calls construct another.
class LaneEntry {
public:
// budget_ms <= 0 waits indefinitely. Pass the output of
@@ -106,8 +114,20 @@ class LaneEntry {
// Deliberately immovable rather than carefully movable. A moved-from entry
// would have to stop releasing the lane while the lane still records it as
// occupied, and that hazard is not worth the convenience: the lane can only
// be recovered by whoever took it. Callers needing to extend the lifetime
// past a scope should hold the entry indirectly instead.
// be recovered by whoever took it.
//
// To hold a lane for longer than one scope, construct the entry in place
// instead of moving one in. Two shapes work:
// std::optional<LaneEntry> held; // member or local
// held.emplace(lane, budget_ms); // takes the lane, held.reset() gives it back
// auto held = std::make_unique<LaneEntry>(lane, budget_ms); // also returnable
// Both outlive the acquiring scope and still release exactly once, when they
// are reset or destroyed. Prefer the optional for a member whose lifetime is
// the handler's; use the unique_ptr when the entry has to be returned, since
// an optional of an immovable type is itself immovable and cannot be. A
// factory may instead write `return LaneEntry(lane, budget_ms);`, which C++17
// guarantees to elide, whereas `LaneEntry entry(...); return entry;` does not
// compile, because that form is a move.
LaneEntry(LaneEntry &&) = delete;
LaneEntry &operator=(LaneEntry &&) = delete;

22
backend/cpp/audio-cpp/inference_lane_test.cpp
View File

@@ -292,6 +292,15 @@ static void test_bounded_wait_times_out() {
// The waiter arrives immediately, so the holder's elapsed time is far below
// the budget and the fail-fast path must not trigger here.
//
// Load-sensitive margin, and the tightest one in this file: what makes this
// the timeout path rather than the fail-fast path is the holder's age
// staying under 120 ms at the arrival check. All that sits between the
// holder's stamp and this call is one signal handover, microseconds against
// a 120 ms allowance, but unlike the other margins here load pushes this one
// toward failing rather than away from it. If it ever does flip, the symptom
// is the wording assertions below going red, not a hang, and the fix is a
// larger budget rather than a weaker assertion.
const EntryOutcome outcome = try_entry(lane, kBudgetMs);
release.raise();
holder.join();
@@ -543,12 +552,23 @@ static void test_failure_messages_are_diagnosable() {
});
check(held.await(5000), "B10 holder took the lane");
// The two budgets have to straddle the run's age, or both callers take the
// same path and the comparisons below are between two fail-fast messages
// that differ only in the budget they print.
//
// Margin: 30 ms is far under the ~120 ms age, and load only ages the run
// further, so `fast` fails fast. 400 ms is far over it, with the same
// slack and the same safe direction as the B3 test, so `slow` queues and
// then times out.
nap(120);
const EntryOutcome fast = try_entry(lane, 30);
const EntryOutcome slow = try_entry(lane, 60);
const EntryOutcome slow = try_entry(lane, 400);
release.raise();
holder.join();
check(mentions(fast.message, kStillRunningPhrase) &&
mentions(slow.message, kTimedOutPhrase),
"B10 one caller took the fail-fast path and the other timed out");
check(fast.message != slow.message,
"B10 the two failure modes do not share one message");
check(!fast.message.empty() && !slow.message.empty(),