From 5d12da49ad0bef096f0e1c814e309e79384172b5 Mon Sep 17 00:00:00 2001 From: Bhagirath Mehta Date: Thu, 18 Jun 2026 17:54:04 -0500 Subject: [PATCH 1/5] devirt: mark concrete leaf impl classes final Mark the leaf concrete implementations of IHttpClient and IOfflineStorage final so the compiler can devirtualize (and often inline) calls made through them: HttpClient_{Curl,WinInet,WinRt,Apple,Android,CAPI} and OfflineStorage_Room / MemoryStorage / OfflineStorageHandler. Each was verified to have no subclass anywhere in the tree (lib + tests + wrappers). Deliberately NOT marked: - OfflineStorage_SQLite -- tests/unittests/OfflineStorageTests_SQLite.cpp subclasses it (OfflineStorage_SQLiteNoAutoCommit). - TelemetrySystemBase -- base of TelemetrySystem / AITelemetrySystem. Validated: NDK aarch64 -fsyntax-only on OfflineStorage_Room, OfflineStorageHandler and MemoryStorage. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- lib/http/HttpClient_Android.hpp | 2 +- lib/http/HttpClient_Apple.hpp | 2 +- lib/http/HttpClient_CAPI.hpp | 2 +- lib/http/HttpClient_Curl.hpp | 2 +- lib/http/HttpClient_WinInet.hpp | 2 +- lib/http/HttpClient_WinRt.hpp | 2 +- lib/offline/MemoryStorage.hpp | 2 +- lib/offline/OfflineStorageHandler.hpp | 2 +- lib/offline/OfflineStorage_Room.hpp | 2 +- 9 files changed, 9 insertions(+), 9 deletions(-) diff --git a/lib/http/HttpClient_Android.hpp b/lib/http/HttpClient_Android.hpp index ad7905eca..4a2839c7e 100644 --- a/lib/http/HttpClient_Android.hpp +++ b/lib/http/HttpClient_Android.hpp @@ -12,7 +12,7 @@ namespace MAT_NS_BEGIN { - class HttpClient_Android : public IHttpClient + class HttpClient_Android final : public IHttpClient { enum class RequestState : int8_t { diff --git a/lib/http/HttpClient_Apple.hpp b/lib/http/HttpClient_Apple.hpp index e9b22b1f0..a98eee238 100644 --- a/lib/http/HttpClient_Apple.hpp +++ b/lib/http/HttpClient_Apple.hpp @@ -13,7 +13,7 @@ namespace MAT_NS_BEGIN { - class HttpClient_Apple : public IHttpClient { + class HttpClient_Apple final : public IHttpClient { public: HttpClient_Apple(); virtual ~HttpClient_Apple() noexcept; diff --git a/lib/http/HttpClient_CAPI.hpp b/lib/http/HttpClient_CAPI.hpp index 5fb3cc088..61af911e8 100644 --- a/lib/http/HttpClient_CAPI.hpp +++ b/lib/http/HttpClient_CAPI.hpp @@ -13,7 +13,7 @@ namespace MAT_NS_BEGIN { - class HttpClient_CAPI : public IHttpClient { + class HttpClient_CAPI final : public IHttpClient { public: HttpClient_CAPI(http_send_fn_t sendFn, http_cancel_fn_t cancelFn); diff --git a/lib/http/HttpClient_Curl.hpp b/lib/http/HttpClient_Curl.hpp index b1bb5344c..b441c7449 100644 --- a/lib/http/HttpClient_Curl.hpp +++ b/lib/http/HttpClient_Curl.hpp @@ -47,7 +47,7 @@ namespace MAT_NS_BEGIN { /** * Curl-based HTTP client */ -class HttpClient_Curl : public IHttpClient { +class HttpClient_Curl final : public IHttpClient { public: HttpClient_Curl(); virtual ~HttpClient_Curl(); diff --git a/lib/http/HttpClient_WinInet.hpp b/lib/http/HttpClient_WinInet.hpp index 7e9379ded..77fd34a48 100644 --- a/lib/http/HttpClient_WinInet.hpp +++ b/lib/http/HttpClient_WinInet.hpp @@ -20,7 +20,7 @@ typedef void* HINTERNET; class WinInetRequestWrapper; -class HttpClient_WinInet : public IHttpClient { +class HttpClient_WinInet final : public IHttpClient { public: // Common IHttpClient methods HttpClient_WinInet(); diff --git a/lib/http/HttpClient_WinRt.hpp b/lib/http/HttpClient_WinRt.hpp index e6352a45b..d6916e617 100644 --- a/lib/http/HttpClient_WinRt.hpp +++ b/lib/http/HttpClient_WinRt.hpp @@ -28,7 +28,7 @@ namespace MAT_NS_BEGIN { class WinRtRequestWrapper; -class HttpClient_WinRt : public IHttpClient { +class HttpClient_WinRt final : public IHttpClient { public: HttpClient_WinRt(); virtual ~HttpClient_WinRt(); diff --git a/lib/offline/MemoryStorage.hpp b/lib/offline/MemoryStorage.hpp index 8a378dc5d..64f3f7b8c 100644 --- a/lib/offline/MemoryStorage.hpp +++ b/lib/offline/MemoryStorage.hpp @@ -24,7 +24,7 @@ namespace MAT_NS_BEGIN { - class MemoryStorage : public IOfflineStorage + class MemoryStorage final : public IOfflineStorage { public: diff --git a/lib/offline/OfflineStorageHandler.hpp b/lib/offline/OfflineStorageHandler.hpp index e7bdce4cb..c32de9ad8 100644 --- a/lib/offline/OfflineStorageHandler.hpp +++ b/lib/offline/OfflineStorageHandler.hpp @@ -23,7 +23,7 @@ namespace MAT_NS_BEGIN { - class OfflineStorageHandler : public IOfflineStorage, public IOfflineStorageObserver + class OfflineStorageHandler final : public IOfflineStorage, public IOfflineStorageObserver { public: OfflineStorageHandler(ILogManager& logManager, IRuntimeConfig& runtimeConfig, ITaskDispatcher& taskDispatcher); diff --git a/lib/offline/OfflineStorage_Room.hpp b/lib/offline/OfflineStorage_Room.hpp index 71eee31b5..4cb194f09 100644 --- a/lib/offline/OfflineStorage_Room.hpp +++ b/lib/offline/OfflineStorage_Room.hpp @@ -23,7 +23,7 @@ namespace MAT_NS_BEGIN { - class OfflineStorage_Room : public IOfflineStorage + class OfflineStorage_Room final : public IOfflineStorage { protected: class ConnectedEnv From d0c0afb295f0de5b29802c8a15110cb88182ee51 Mon Sep 17 00:00:00 2001 From: Bhagirath Mehta Date: Fri, 19 Jun 2026 01:45:10 -0500 Subject: [PATCH 2/5] Internal linkage: mark TU-local C-API helpers and DllMain global static Companion to the `final` devirtualization in this PR: give internal linkage to translation-unit-local symbols so the compiler can inline / drop them and keep them out of the (static-archive and shared-object) symbol table. This helps the static-lib consumption path that -fvisibility=hidden does not fully cover, since hidden visibility only trims the dynamic export table while these symbols keep external linkage across TUs. lib/api/capi.cpp: mark the 10 file-local C-API dispatch helpers static (remove_client, mat_open_core, mat_open, mat_open_with_params, mat_log, mat_close, mat_pause, mat_resume, mat_upload, mat_flushAndTeardown). Verified each is called only from the single exported entry point evt_api_call_default (and each other) within capi.cpp, and appears in no header and no other translation unit. capi_get_client stays external (it is MAT::capi_get_client, declared in a header and used by the CAPI HTTP client). Consistent with the file's existing static mtx/clients. lib/shared/dllmain.cpp: mark thread_count static -- a file-scope mutable global mutated only inside DllMain in this TU. get_platform_uuid (sysinfo_sources.cpp) was deliberately NOT touched: it has no caller in-tree, so marking it static would trip -Wunused-function under -Werror. Verified: NDK clang aarch64 -fsyntax-only on capi.cpp is clean. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- lib/api/capi.cpp | 20 ++++++++++---------- lib/shared/dllmain.cpp | 2 +- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/lib/api/capi.cpp b/lib/api/capi.cpp index 916c4ebda..6125cd68c 100644 --- a/lib/api/capi.cpp +++ b/lib/api/capi.cpp @@ -49,7 +49,7 @@ capi_client * MAT::capi_get_client(evt_handle_t handle) /// /// Remove C API handle from active client tracking struct. /// -void remove_client(evt_handle_t handle) +static void remove_client(evt_handle_t handle) { LOCKGUARD(mtx); clients.erase(handle); @@ -66,7 +66,7 @@ void remove_client(evt_handle_t handle) return ENOENT; \ }; -evt_status_t mat_open_core( +static evt_status_t mat_open_core( evt_context_t *ctx, const char* config, http_send_fn_t httpSendFn, @@ -175,7 +175,7 @@ evt_status_t mat_open_core( return ctx->result; } -evt_status_t mat_open(evt_context_t *ctx) +static evt_status_t mat_open(evt_context_t *ctx) { if (ctx == nullptr) { @@ -186,7 +186,7 @@ evt_status_t mat_open(evt_context_t *ctx) return mat_open_core(ctx, config, nullptr, nullptr, nullptr, nullptr, nullptr); } -evt_status_t mat_open_with_params(evt_context_t *ctx) +static evt_status_t mat_open_with_params(evt_context_t *ctx) { if (ctx == nullptr) { @@ -233,7 +233,7 @@ evt_status_t mat_open_with_params(evt_context_t *ctx) /** * Marashal C struct to C++ API */ -evt_status_t mat_log(evt_context_t *ctx) +static evt_status_t mat_log(evt_context_t *ctx) { VERIFY_CLIENT_HANDLE(client, ctx); @@ -285,7 +285,7 @@ evt_status_t mat_log(evt_context_t *ctx) return ctx->result; } -evt_status_t mat_close(evt_context_t *ctx) +static evt_status_t mat_close(evt_context_t *ctx) { VERIFY_CLIENT_HANDLE(client, ctx); const auto result = static_cast(LogManagerProvider::Release(client->logmanager->GetLogConfiguration())); @@ -305,7 +305,7 @@ evt_status_t mat_close(evt_context_t *ctx) return result; } -evt_status_t mat_pause(evt_context_t *ctx) +static evt_status_t mat_pause(evt_context_t *ctx) { VERIFY_CLIENT_HANDLE(client, ctx); const auto result = static_cast(client->logmanager->PauseTransmission()); @@ -313,7 +313,7 @@ evt_status_t mat_pause(evt_context_t *ctx) return result; } -evt_status_t mat_resume(evt_context_t *ctx) +static evt_status_t mat_resume(evt_context_t *ctx) { VERIFY_CLIENT_HANDLE(client, ctx); const auto result = static_cast(client->logmanager->ResumeTransmission()); @@ -321,7 +321,7 @@ evt_status_t mat_resume(evt_context_t *ctx) return result; } -evt_status_t mat_upload(evt_context_t *ctx) +static evt_status_t mat_upload(evt_context_t *ctx) { VERIFY_CLIENT_HANDLE(client, ctx); const auto result = static_cast(client->logmanager->UploadNow()); @@ -329,7 +329,7 @@ evt_status_t mat_upload(evt_context_t *ctx) return result; } -evt_status_t mat_flushAndTeardown(evt_context_t *ctx) +static evt_status_t mat_flushAndTeardown(evt_context_t *ctx) { VERIFY_CLIENT_HANDLE(client, ctx); client->logmanager->FlushAndTeardown(); diff --git a/lib/shared/dllmain.cpp b/lib/shared/dllmain.cpp index 4cfa868c6..d584c56c6 100644 --- a/lib/shared/dllmain.cpp +++ b/lib/shared/dllmain.cpp @@ -18,7 +18,7 @@ #ifdef _MANAGED #pragma unmanaged #endif -unsigned thread_count = 0; +static unsigned thread_count = 0; BOOL APIENTRY DllMain(HMODULE /* hModule */, DWORD ul_reason_for_call, LPVOID /* lpReserved */) { From 693926cdbc8e7c639dc4957ed06febe24634c447 Mon Sep 17 00:00:00 2001 From: Bhagirath Mehta Date: Wed, 24 Jun 2026 12:39:42 -0500 Subject: [PATCH 3/5] Fix typo in capi.cpp comment: Marashal -> Marshal Addresses Copilot review comment. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- lib/api/capi.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/api/capi.cpp b/lib/api/capi.cpp index 6125cd68c..cfe6e0a0e 100644 --- a/lib/api/capi.cpp +++ b/lib/api/capi.cpp @@ -231,7 +231,7 @@ static evt_status_t mat_open_with_params(evt_context_t *ctx) } /** - * Marashal C struct to C++ API + * Marshal C struct to C++ API */ static evt_status_t mat_log(evt_context_t *ctx) { From 4f1718ed6b8862043938277e4b446bed36bd5d84 Mon Sep 17 00:00:00 2001 From: Bhagirath Mehta Date: Tue, 30 Jun 2026 09:56:00 -0500 Subject: [PATCH 4/5] Narrow scope: keep 'final' only where it measurably devirtualizes Per review feedback (lalitb), measured the devirtualization effect of the 'final' markers (vendored Linux shared libmat.so, -O2 -ffunction-sections -fvisibility=hidden -Wl,--gc-sections, no LTO), base vs this PR: - Final binary size: NO change (.text byte-size identical). Devirtualization swaps an indirect call for a same-width direct call -- perf, not size. - 'final' devirtualizes only where the concrete type is known: object-level indirect-call counts dropped in OfflineStorageHandler.o (70->64) and MemoryStorage.o (8->3) -- a class calling its own virtual methods and OfflineStorageHandler's concrete MemoryStorage member. Call sites routed through IHttpClient/IOfflineStorage base references were unchanged (LogManagerImpl/TelemetrySystem/OfflineStorage_SQLite/HttpClient_Curl TUs byte-identical). So 'final' on the HttpClient_* clients and OfflineStorage_Room bought no measured devirtualization while still restricting subclassing. Drop it from those; keep it only on MemoryStorage and OfflineStorageHandler (internal lib/offline impl types, not extension points), where it does help. The internal-linkage (static) cleanups in capi.cpp and dllmain.cpp are orthogonal and retained. Files: lib/http/HttpClient_{Android,Apple,CAPI,Curl,WinInet,WinRt}.hpp, lib/offline/OfflineStorage_Room.hpp Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- lib/http/HttpClient_Android.hpp | 2 +- lib/http/HttpClient_Apple.hpp | 2 +- lib/http/HttpClient_CAPI.hpp | 2 +- lib/http/HttpClient_Curl.hpp | 2 +- lib/http/HttpClient_WinInet.hpp | 2 +- lib/http/HttpClient_WinRt.hpp | 2 +- lib/offline/OfflineStorage_Room.hpp | 2 +- 7 files changed, 7 insertions(+), 7 deletions(-) diff --git a/lib/http/HttpClient_Android.hpp b/lib/http/HttpClient_Android.hpp index 4a2839c7e..ad7905eca 100644 --- a/lib/http/HttpClient_Android.hpp +++ b/lib/http/HttpClient_Android.hpp @@ -12,7 +12,7 @@ namespace MAT_NS_BEGIN { - class HttpClient_Android final : public IHttpClient + class HttpClient_Android : public IHttpClient { enum class RequestState : int8_t { diff --git a/lib/http/HttpClient_Apple.hpp b/lib/http/HttpClient_Apple.hpp index a98eee238..e9b22b1f0 100644 --- a/lib/http/HttpClient_Apple.hpp +++ b/lib/http/HttpClient_Apple.hpp @@ -13,7 +13,7 @@ namespace MAT_NS_BEGIN { - class HttpClient_Apple final : public IHttpClient { + class HttpClient_Apple : public IHttpClient { public: HttpClient_Apple(); virtual ~HttpClient_Apple() noexcept; diff --git a/lib/http/HttpClient_CAPI.hpp b/lib/http/HttpClient_CAPI.hpp index 61af911e8..5fb3cc088 100644 --- a/lib/http/HttpClient_CAPI.hpp +++ b/lib/http/HttpClient_CAPI.hpp @@ -13,7 +13,7 @@ namespace MAT_NS_BEGIN { - class HttpClient_CAPI final : public IHttpClient { + class HttpClient_CAPI : public IHttpClient { public: HttpClient_CAPI(http_send_fn_t sendFn, http_cancel_fn_t cancelFn); diff --git a/lib/http/HttpClient_Curl.hpp b/lib/http/HttpClient_Curl.hpp index b441c7449..b1bb5344c 100644 --- a/lib/http/HttpClient_Curl.hpp +++ b/lib/http/HttpClient_Curl.hpp @@ -47,7 +47,7 @@ namespace MAT_NS_BEGIN { /** * Curl-based HTTP client */ -class HttpClient_Curl final : public IHttpClient { +class HttpClient_Curl : public IHttpClient { public: HttpClient_Curl(); virtual ~HttpClient_Curl(); diff --git a/lib/http/HttpClient_WinInet.hpp b/lib/http/HttpClient_WinInet.hpp index 77fd34a48..7e9379ded 100644 --- a/lib/http/HttpClient_WinInet.hpp +++ b/lib/http/HttpClient_WinInet.hpp @@ -20,7 +20,7 @@ typedef void* HINTERNET; class WinInetRequestWrapper; -class HttpClient_WinInet final : public IHttpClient { +class HttpClient_WinInet : public IHttpClient { public: // Common IHttpClient methods HttpClient_WinInet(); diff --git a/lib/http/HttpClient_WinRt.hpp b/lib/http/HttpClient_WinRt.hpp index d6916e617..e6352a45b 100644 --- a/lib/http/HttpClient_WinRt.hpp +++ b/lib/http/HttpClient_WinRt.hpp @@ -28,7 +28,7 @@ namespace MAT_NS_BEGIN { class WinRtRequestWrapper; -class HttpClient_WinRt final : public IHttpClient { +class HttpClient_WinRt : public IHttpClient { public: HttpClient_WinRt(); virtual ~HttpClient_WinRt(); diff --git a/lib/offline/OfflineStorage_Room.hpp b/lib/offline/OfflineStorage_Room.hpp index 4cb194f09..71eee31b5 100644 --- a/lib/offline/OfflineStorage_Room.hpp +++ b/lib/offline/OfflineStorage_Room.hpp @@ -23,7 +23,7 @@ namespace MAT_NS_BEGIN { - class OfflineStorage_Room final : public IOfflineStorage + class OfflineStorage_Room : public IOfflineStorage { protected: class ConnectedEnv From 9b250ee554e48d7f631c3fefdeee5f42d86e9adc Mon Sep 17 00:00:00 2001 From: Bhagirath Mehta Date: Tue, 30 Jun 2026 10:18:48 -0500 Subject: [PATCH 5/5] Drop redundant virtual on non-override methods in final storage classes Newer Clang (macOS-latest, LLVM 19+) enables -Wunnecessary-virtual-specifier, which errors under -Werror when a 'virtual' method that does not override a base method lives inside a 'final' class (it can never be overridden). Marking OfflineStorageHandler and MemoryStorage final left three such methods (DeleteRecordsByKeys, isKilled, GetReservedCount) still declared virtual, breaking the macOS debug build. Remove the now-redundant virtual specifier; these become non-virtual, consistent with the devirtualization goal. No subclasses or overrides of these methods exist. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- lib/offline/MemoryStorage.hpp | 2 +- lib/offline/OfflineStorageHandler.hpp | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/offline/MemoryStorage.hpp b/lib/offline/MemoryStorage.hpp index 64f3f7b8c..32dc82bdf 100644 --- a/lib/offline/MemoryStorage.hpp +++ b/lib/offline/MemoryStorage.hpp @@ -69,7 +69,7 @@ namespace MAT_NS_BEGIN { virtual size_t GetRemainingRecordCountForShutdown() const override; - virtual size_t GetReservedCount(); + size_t GetReservedCount(); virtual std::vector GetRecords(bool shutdown = false, EventLatency minLatency = EventLatency_Unspecified, unsigned maxCount = 0) override; diff --git a/lib/offline/OfflineStorageHandler.hpp b/lib/offline/OfflineStorageHandler.hpp index c32de9ad8..1b07d5d1d 100644 --- a/lib/offline/OfflineStorageHandler.hpp +++ b/lib/offline/OfflineStorageHandler.hpp @@ -64,7 +64,7 @@ namespace MAT_NS_BEGIN { virtual void OnStorageRecordsSaved(size_t numRecords) override; protected: - virtual void DeleteRecordsByKeys(const std::list & keys); + void DeleteRecordsByKeys(const std::list & keys); IOfflineStorageObserver * m_observer; ILogManager & m_logManager; @@ -75,7 +75,7 @@ namespace MAT_NS_BEGIN { KillSwitchManager m_killSwitchManager; ClockSkewManager m_clockSkewManager; - virtual bool isKilled(StorageRecord const& record); + bool isKilled(StorageRecord const& record); std::mutex m_flushLock; bool m_flushPending;