Devirtualize storage hot path: mark MemoryStorage/OfflineStorageHandler final + TU-local statics

[bmehta001]

Devirtualization: mark the two storage hot-path classes final

Marks the concrete leaf storage impls MemoryStorage and OfflineStorageHandler
final so the compiler can devirtualize (and often inline) virtual calls where the
concrete type is known.

Measured effect (this informed the scope)

Built vendored Linux shared libmat.so with -O2 -ffunction-sections -fdata-sections -fvisibility=hidden -Wl,--gc-sections (no LTO), base vs this PR:

• Final binary size: no change. Stripped libmat.so and .text are identical in
  size (186,514 B .text on both). Devirtualization swaps an indirect call *(vtable)
  for a same-width direct call rel32, so it is a perf change, not a size one.

• final only devirtualizes concrete-typed call sites (object-level indirect-call
  counts):

  TU	base → PR
  OfflineStorageHandler.o	70 → 64 (−6)
  MemoryStorage.o	8 → 3 (−5)
  OfflineStorage_SQLite.o, LogManagerImpl.o, TelemetrySystem.o, HttpClient_Curl.o	unchanged

  The eliminated calls are a final class calling its own virtual methods (e.g.
  OfflineStorageHandler → DeleteRecords/ReleaseRecords/DeleteRecordsByKeys/Flush)
  and OfflineStorageHandler's concrete MemoryStorage member. Calls routed through
  IHttpClient/IOfflineStorage base references are not devirtualized.

Scope

Per review feedback, final is kept only on MemoryStorage and OfflineStorageHandler
— internal lib/offline/ impl types (not public extension points) — where it measurably
devirtualizes. It was dropped from HttpClient_Curl/WinInet/WinRt/Apple/Android/CAPI
and OfflineStorage_Room, where it bought no measured devirtualization while still
restricting subclassing.

Deliberately not marked: OfflineStorage_SQLite (subclassed by
OfflineStorage_SQLiteNoAutoCommit in tests), TelemetrySystemBase (base of
TelemetrySystem/AITelemetrySystem).

---

Internal linkage (static) — retained

Orthogonal hygiene: give internal linkage to translation-unit-local symbols so the
compiler can inline/drop them and keep them out of the symbol table. This helps the
static-library consumption path (e.g. vcpkg); -fvisibility=hidden only trims a
shared object's dynamic export table.

• lib/api/capi.cpp — the 10 file-local C-API dispatch helpers (remove_client,
  mat_open_core, mat_open, mat_open_with_params, mat_log, mat_close, mat_pause,
  mat_resume, mat_upload, mat_flushAndTeardown), each called only within capi.cpp
  and in no header/other TU. MAT::capi_get_client stays external.
• lib/shared/dllmain.cpp — thread_count (file-scope global mutated only in DllMain).

Deliberately not touched: get_platform_uuid (sysinfo_sources.cpp) has no in-tree
caller, so static would trip -Wunused-function under -Werror.

Verified: NDK clang aarch64 -fsyntax-only clean; whole-tree cross-reference confirms none
of the internalized symbols are referenced from any header or other TU.

Co-authored-by: Copilot 223556219+Copilot@users.noreply.github.com

[Copilot]

Pull request overview

This PR optimizes hot-path virtual dispatch by marking leaf concrete implementations of the IHttpClient and IOfflineStorage interfaces as final, enabling better compiler devirtualization/inlining in the SDK’s HTTP upload and offline-storage code paths.

Changes:

• Mark concrete HTTP client implementations (HttpClient_*) as final.
• Mark concrete offline storage implementations/handler (OfflineStorage_*, MemoryStorage, OfflineStorageHandler) as final.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
lib/offline/OfflineStorageHandler.hpp	Marks OfflineStorageHandler as final to enable devirtualization through known concrete type usage.
lib/offline/OfflineStorage_Room.hpp	Marks OfflineStorage_Room as final to allow devirtualization in Android Room-backed storage path.
lib/offline/MemoryStorage.hpp	Marks MemoryStorage as final to enable devirtualization for in-memory storage operations.
lib/http/HttpClient_WinRt.hpp	Marks HttpClient_WinRt as final to improve call-site optimization on WinRT HTTP path.
lib/http/HttpClient_WinInet.hpp	Marks HttpClient_WinInet as final to improve call-site optimization on WinInet HTTP path.
lib/http/HttpClient_Curl.hpp	Marks HttpClient_Curl as final to improve call-site optimization on Curl HTTP path.
lib/http/HttpClient_CAPI.hpp	Marks HttpClient_CAPI as final to improve call-site optimization for C API-backed HTTP client.
lib/http/HttpClient_Apple.hpp	Marks HttpClient_Apple as final to improve call-site optimization on Apple HTTP path.
lib/http/HttpClient_Android.hpp	Marks HttpClient_Android as final to improve call-site optimization on Android HTTP path.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.

[lalitb]

I believe most of our storage/upload calls go through IHttpClient / IOfflineStorage references after construction. Do we have compiler output, size data, or a concrete call path showing where this actually devirtualizes without LTO?

My only concern is the tradeoff here: if this does not actually devirtualize the main hot paths, we may be restricting downstream/internal users from subclassing these concrete types without much benefit.

[Copilot]

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated no new comments.

[bmehta001]

@lalitb, You are right. Devirtualization is only happening on concrete-typed call sites. Object-level indirect-call counts:
•  OfflineStorageHandler.o : 70 → 64 (−6)
•  MemoryStorage.o : 8 → 3 (−5)
•  OfflineStorage_SQLite.o ,  LogManagerImpl.o ,  TelemetrySystem.o ,  HttpClient_Curl.o : unchanged
The eliminated calls are a final  class calling its own virtual methods (e.g.  OfflineStorageHandler  →  DeleteRecords  /  ReleaseRecords  /  DeleteRecordsByKeys  /  Flush ) plus  OfflineStorageHandler 's concrete  MemoryStorage  member. Everything routed through  IHttpClient  /  IOfflineStorage  base references is unaffected.

My main reason for my recent series of PRs is performance (slight impact) and decreasing binary size (no impact), so none of this PR is actually necessary. I will narrow this PR to keep final only where it actually devirtualizes: MemoryStorage and OfflineStorageHandler, and drop it from the  HttpClient_*  /  OfflineStorage_Room  /  dllmain  classes where it changed nothing, so we're not restricting subclassing for no benefit.