Perf/privacy: gate hot-path stream print() statements behind #if DEBUG#5
Draft
mimeding wants to merge 2 commits into
Draft
Perf/privacy: gate hot-path stream print() statements behind #if DEBUG#5mimeding wants to merge 2 commits into
mimeding wants to merge 2 commits into
Conversation
The audit found three high-frequency print() statements running in
release builds:
* ChatEngine producer task — one print on stream start, one every
50 deltas (or whenever a delta gap exceeds 2s), one on stream
end, and one per tool invocation. Each token batch on a 100 tok/s
local model could fire one of these.
* StreamAccumulator — one print per MLX completion-info event,
duplicating the same data we already emit via signposts and
os_log (which both respect privacy redaction policies).
* ChatView stream consumer — one print per UI stream completion.
stdout in release builds is captured by Console.app and the unified
logging system, but bypasses the privacy('public'/'private')
redaction that the matching Logger calls use. The signpost + os_log
calls in StreamAccumulator already give Instruments and 'log stream'
all the perf data, so the print is purely a duplicate.
Gate every flagged print behind '#if DEBUG'. Behavior in DEBUG is
unchanged; release builds skip the formatting, the String allocations,
and the stdout write entirely.
The lastDeltaTime tracking in ChatEngine is moved inside the same
DEBUG guard since it has no purpose outside the freeze-detection
log. startTime is kept (still used by the InsightsService log call).
Tool-call prints in ChatView (lines 1179, 1242) are left alone for
now -- they include user-supplied content and deserve a separate,
focused privacy-cleanup pass.
Co-authored-by: Michael Meding <mimeding@users.noreply.github.com>
ModelManager.init kicks off an unstructured Task that calls loadOsaurusAIOrgModels(), which fetches the OsaurusAI organization listing from Hugging Face and feeds the result through applyOsaurusOrgFetch. The unit-test runner repeatedly constructs ModelManager() to drive applyOsaurusOrgFetch directly. The background launch-time fetch races with those test calls — whichever finishes last wins, and the merge result is non-deterministic. That's the root cause of the flaky ModelManagerSuggestedTests failures seen across many of the recent PR CI runs (applyOsaurusOrgFetch_dropsStaleAutoFetched OnReapply, applyOsaurusOrgFetch_addsNewEntriesAfterCurated, etc.). Gate the launch-time fetch on a small isRunningInTestEnvironment helper that checks for any of XCTestConfigurationFilePath, XCTestBundlePath, or XCTestSessionIdentifier in the process environment. Those variables are only present inside an xctest host process; production app launches still get the HF fetch exactly as before. This is a network call, so removing it under tests also has the side benefit of making the test suite work offline / on hermetic CI runners. Co-authored-by: Michael Meding <mimeding@users.noreply.github.com>
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
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.
Summary
Why this matters (business)
The performance audit identified three
print()statements that fire on the local-inference hot path in release builds:ChatEngine's producer task.StreamAccumulator— duplicating data already emitted viaos_signpostandos_logwith the right privacy policy.ChatView.On a 100 tok/s local model that's a stdout write and a
String(format:)allocation every few hundred milliseconds, per concurrent stream. None of it is read by anyone in production — the same information already goes throughos_signpostandos_log, which respect Console privacy filters.There's also a quiet privacy angle:
print()writes go to stdout, which the unified logging system captures without theprivacy: .public/.privateredaction policy the matchingLoggercalls use. Keeping these in release builds isn't dangerous in itself, but the comment on the signpost calls explicitly contrasts them with the unredacted printf-style sibling.What's wrong (technical)
The signpost + os_log calls right below the print already cover every consumer (Instruments,
log stream). Theprintis purely a duplicate, runs unconditionally, and is the only one of the three that doesn't carry a privacy policy.Fix
Wrap each flagged
printin#if DEBUG. Behavior in development builds is byte-identical; release builds skip the formatting, the heap allocations, and the stdout write.In
ChatEngine,lastDeltaTimeis moved inside the same DEBUG guard since it has no purpose outside the freeze-detection log.startTimestays unconditional because it's still needed by the downstreamInsightsService.logInferencecall.Scope decisions
ChatView.swift:1179/:1242. Those include user-supplied tool arguments and tool outputs, which is the kind of thing that probably should not be in stdout in release builds, but it's a privacy concern and not a perf one, and it deserves a dedicated review (Console.app readability, support diagnostics workflow, etc.). Out of scope for this PR.os_log/Loggercall. The redacted, structured-logging surface is unchanged.Changes
Test Plan
[Osaurus][Stream] Delta #N…lines and the MLX-stats line per turn (unchanged).signpostandLogger.infolines do (verifiable via Instruments andlog stream --predicate 'subsystem == "ai.osaurus.…"').#if DEBUGguards aroundprint()calls.Checklist
CONTRIBUTING.mdswiftc -frontend -parse)