Skip to content

Server: avoid per-frame deep copy of audio buffers in CreateLevelsForAllConChannels#3803

Open
mcfnord wants to merge 1 commit into
jamulussoftware:mainfrom
mcfnord:fix/levels-pass-by-const-ref
Open

Server: avoid per-frame deep copy of audio buffers in CreateLevelsForAllConChannels#3803
mcfnord wants to merge 1 commit into
jamulussoftware:mainfrom
mcfnord:fix/levels-pass-by-const-ref

Conversation

@mcfnord

@mcfnord mcfnord commented Jul 18, 2026

Copy link
Copy Markdown
Contributor

What

CServer::CreateLevelsForAllConChannels() declares its vecvecsData parameter as

const CVector<CVector<int16_t>> vecvecsData   // by value

This PR changes it to const CVector<CVector<int16_t>>& (by const reference), matching the neighbouring vecNumAudioChannels parameter.

Why

The function is called from CServer::OnTimer() — the realtime audio timer thread — on every frame in which at least one client is connected:

const bool bSendChannelLevels = CreateLevelsForAllConChannels ( iNumClients, vecNumAudioChannels, vecvecsData, vecChannelLevels );

Because CVector derives from std::vector, passing by value deep-copies the entire pre-allocated member vector every frame. vecvecsData is initialized in the CServer constructor to iMaxNumChannels (the -u setting) worst-case stereo buffers and is never shrunk, so the copy always covers the full configured channel limit — even a nearly empty server pays it in full. This both allocates on the audio thread (which the codebase otherwise takes care to avoid — see the "no memory must be allocated" comments in the CServer constructor) and burns memory bandwidth.

A small allocation-counting reproduction of the exact copy (150 channels = -u 150, 2×128 int16 per buffer, 375 frames/s) measured ~56,600 heap allocations/s and ~30 MB/s copied, purely from the by-value parameter; at the default -u 10 it is still ~4,100 allocations/s and ~2 MB/s. The copy is not elided at -O2.

Change

One parameter, declaration + definition — no behavioural change. The function only reads vecvecsData.

Testing

  • Headless build (CONFIG+=headless, Qt 5.15) is clean.
  • Server starts, runs, and shuts down cleanly on SIGTERM (OnHandledSignal: 15).
  • clang-format-14 applied to the touched files.

See #3804 for the analysis.

CServer::CreateLevelsForAllConChannels() took its vecvecsData argument
(CVector<CVector<int16_t>>, i.e. one audio buffer per connected client)
by value. The function is called from CServer::OnTimer() on the realtime
audio timer thread on every frame that has connected clients, so the
entire per-client audio data was deep-copied each frame.

Pass it by const reference, matching the adjacent vecNumAudioChannels
parameter. No behavioural change.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@ann0see

ann0see commented Jul 18, 2026

Copy link
Copy Markdown
Member

Seems sensible. We'd need to check if there's really no reason for a copy though.

@mcfnord

mcfnord commented Jul 18, 2026

Copy link
Copy Markdown
Contributor Author

Not sure how to parse the scope of const here, but it might be a clue that perhaps there's no reason to copy the bits.

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