Skip to content

perf: set the database charset once per connection in sbpp_main - #82

Merged
Rushaway merged 2 commits into
mainfrom
perf/sbpp-admincache
Oct 29, 2026
Merged

Rushaway merged 2 commits into
mainfrom
perf/sbpp-admincache

Conversation

@Rushaway

@Rushaway Rushaway commented Oct 28, 2026 •

Copy link
Copy Markdown
Member

Summary

On css-ze, our production profiler shows sbpp_main::OnRebuildAdminCache at 13.5 ms per call (p50 12 ms, p99 24.4 ms, 15.9 s of main-thread time over 7 days, 1179 calls). It runs about three times per map change (overrides, groups, admins) and on every sm_reloadadmins. Each call stalls the server for close to a full tick (15 ms at 66 tick), and the p99 calls take more than a tick.

When the database is already connected, OnRebuildAdminCache() calls GotDatabase(DB, "", 0) again for each cache part. GotDatabase() then calls DB.SetCharset("utf8mb4") each time. On MySQL this is a synchronous call on the main thread:

  • the driver takes the connection's full lock first (LockForFullAtomicOperation()), so it waits for any threaded query in flight on that connection, including the queries queued by the previous cache part;
  • then mysql_set_character_set() sends SET NAMES and waits for the reply, a full round trip to the database.

The charset only needs to be set once per connection. mysql_set_character_set() also stores the charset in the connection options, and the SourceMod MySQL driver enables MYSQL_OPT_RECONNECT, so the charset survives the driver's automatic reconnects too. sbpp_main never closes or replaces DB, so the connection stays the same for the life of the plugin.

  • GotDatabase() assigns DB and calls SetCharset() (with the existing async SET NAMES fallback) only when it gets a new handle (db != DB). Calls from OnRebuildAdminCache() with the current handle skip it.
  • Everything else in GotDatabase() still runs on every call, as before: config reset, InsertServerInfo(), the overrides/groups/admins queries, and CheckConnectedClientBans().
  • Version: CHANGELOG entry 2.2.10-srcdslab. The plugin version comes from the release tag (sbpp_version.inc stays dev).

Expected gain: the per-call cost should drop from about 12 to 24 ms to well under 1 ms. That removes about 40 ms of main-thread stall per map change, plus one stall per manual admin reload.

Behaviour

Unchanged. The connection is set to utf8mb4 once when it is established, exactly as before. Later rebuilds reuse the same handle, which already has that charset. The only difference: if SetCharset() fails on the first connect, it is no longer retried on every rebuild. The async SET NAMES utf8mb4 fallback is still sent once on that connection.

Test plan

  • Compiles with spcomp 1.12.0.7259 using the same dependencies as CI (no errors, no warnings)
  • CI build passes
  • On a test server, change map and run sm_reloadadmins. Check that admins, groups and overrides load, that names with UTF-8/emoji are still stored correctly in bans, and that OnRebuildAdminCache drops in the profiler

🤖 Generated with Claude Code

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

@WareZ57 WareZ57 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Read this against the rest of sbpp_main, looks correct to me.

  • DB is only ever assigned in GotDatabase() and is never closed or replaced, and Database.Connect(GotDatabase, ...) is only called while DB is null (OnPluginStart, OnRebuildAdminCache, and the retry timer #83 adds). So db != DB means exactly "this is a new connection", and the three calls per map from OnRebuildAdminCache() with the current handle skip SetCharset().
  • No leak or handle overwrite: there's no path where a second successful connect replaces a live DB.
  • The SET NAMES utf8mb4 fallback still runs once on a new connection if SetCharset() fails, which is fine.
  • Built locally with spcomp 1.12.0.7259 (same -i include as CI): all 6 plugins compile with no errors or warnings, both on this branch alone and merged with #83. sbpp_main.sp merges cleanly with #83; only the two new CHANGELOG.md entries conflict, as you already noted.

LGTM.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@Rushaway
Rushaway merged commit c211709 into main Oct 29, 2026
3 checks passed
@Rushaway
Rushaway deleted the perf/sbpp-admincache branch October 29, 2026 16:10
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