Reduce binary footprint and support embedding the SDK as a CMake subproject#1499
Reduce binary footprint and support embedding the SDK as a CMake subproject#1499bmehta001 wants to merge 30 commits into
Conversation
On the CPP11/curl path (non-Apple, non-Windows), the built-in libcurl HTTP client was always compiled and curl was a hard find_package(CURL REQUIRED) dependency -- pulling in curl and a TLS backend (OpenSSL/mbedTLS) even for hosts that already have their own HTTP stack. Add option(BUILD_CURL_HTTP_CLIENT ON). When OFF, the curl block is skipped (no find_package(CURL), no link, no -DHAVE_MAT_CURL_HTTP_CLIENT) and the build instead defines -DMATSDK_NO_DEFAULT_HTTP_CLIENT. mat/config.h then undefines HAVE_MAT_DEFAULT_HTTP_CLIENT centrally (regardless of the config preset), which the SDK already handles end-to-end: HttpClientFactory and HttpClient_Curl.cpp compile out, and LogManagerImpl's existing !HAVE_MAT_DEFAULT_HTTP_CLIENT branch requires the host to supply an IHttpClient via CFG_MODULE_HTTP_CLIENT. Default ON keeps existing behavior unchanged. Apple/Windows are unaffected (they use native HTTP stacks and never enter the curl block). Validated on WSL x64-linux: with OFF, libmat has no curl symbols and a consumer links with no -lcurl/-lTLS (1.43 MB stripped, vs 4.39 MB with curl+mbedTLS and 10.65 MB with curl+OpenSSL). Files changed: - CMakeLists.txt: BUILD_CURL_HTTP_CLIENT option + gating - lib/include/mat/config.h: central HAVE_MAT_DEFAULT_HTTP_CLIENT opt-out Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…tprint The SDK uses SQLite only for its offline event-storage cache (plain tables, transactions, WAL, autovacuum/VACUUM, a few PRAGMAs, and one custom UTF-8 function), so most SQLite subsystems are dead weight. Add an option to compile a private SQLite from the vendored amalgamation with a set of amalgamation-safe strip flags (single source of truth: MATSDK_SQLITE_MINIMAL_DEFS), removing the external sqlite3 dependency and shrinking SQLite ~10.2% (.text) / ~12.5% (object). - Root CMakeLists.txt: add option(MATSDK_MINIMAL_SQLITE) (default OFF). In vcpkg mode, skip find_package(unofficial-sqlite3) when bundling, and emit a clear FATAL_ERROR pointing at the system-sqlite/minimal-sqlite features when neither provides SQLite (e.g. a bare [core] install). - lib/CMakeLists.txt: define MATSDK_SQLITE_MINIMAL_DEFS, compute MATSDK_BUNDLE_SQLITE (minimal OR vendored-Android), and build a single sqlite3_bundled. The strip flags are applied ONLY when MATSDK_MINIMAL_SQLITE is ON, so the default Android legacy build keeps its existing unstripped bundled SQLite. Warnings are disabled on the vendored target (/w on MSVC, -w on GCC/Clang for the stripped build) so the SDK's -Werror/-WX does not fire on amalgamation code. A static mat propagates the PRIVATE sqlite3_bundled through its link interface, so export+install it. - MSTelemetryConfig.cmake.in: skip find_dependency(unofficial-sqlite3) when bundled. - vcpkg port: add a minimal-sqlite feature (-DMATSDK_MINIMAL_SQLITE=ON) and move sqlite3 into a default system-sqlite feature so [core,minimal-sqlite] drops it. - docs/building-with-vcpkg.md: document the feature, the size win, and the static-absorption symbol-visibility caveat. SQLITE_OMIT_AUTOINIT and SQLITE_DEFAULT_MEMSTATUS=0 are deliberately NOT stripped: the former because skipSqliteInitAndShutdown lets the host skip the SDK's explicit sqlite3_initialize() (which the host cannot do against a private SQLite), the latter because the SDK arms a soft heap limit via sqlite3_soft_heap_limit64() that is only enforced while memory statistics are enabled. Validated: vendored Linux Debug (77 offline-storage/SQLite unit tests pass on the debug amalgamation), vcpkg [core,minimal-sqlite] consumer (links MSTelemetry::sqlite3_bundled, runs 10/10, external sqlite3 dropped), default vcpkg path regression (system-sqlite intact), and MSVC compile/link of sqlite3_bundled+mat. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR introduces opt-in build knobs intended to reduce the SDK’s footprint in host-embedded scenarios by (1) allowing builds without the built-in libcurl HTTP client and (2) optionally replacing the external SQLite dependency with a private, feature-stripped bundled SQLite.
Changes:
- Added
BUILD_CURL_HTTP_CLIENTto omit the built-in curl/TLS HTTP client and require a host-suppliedIHttpClient. - Added
MATSDK_MINIMAL_SQLITEand corresponding vcpkgminimal-sqlitefeature to build a stripped, bundled SQLite instead of depending on vcpkg’ssqlite3. - Updated the vcpkg port and documentation to reflect the new SQLite feature split and minimal-SQLite option.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tools/ports/cpp-client-telemetry/vcpkg.json | Moves sqlite3 into a default system-sqlite feature and adds minimal-sqlite. |
| tools/ports/cpp-client-telemetry/portfile.cmake | Wires vcpkg minimal-sqlite feature into -DMATSDK_MINIMAL_SQLITE=ON. |
| lib/include/mat/config.h | Adds MATSDK_NO_DEFAULT_HTTP_CLIENT override to disable HAVE_MAT_DEFAULT_HTTP_CLIENT. |
| lib/CMakeLists.txt | Implements bundled/minimal SQLite build, link selection, and installation/export logic. |
| docs/building-with-vcpkg.md | Documents the new vcpkg minimal-sqlite feature and behavior. |
| CMakeLists.txt | Adds MATSDK_MINIMAL_SQLITE and BUILD_CURL_HTTP_CLIENT; adjusts vcpkg SQLite discovery flow. |
| cmake/MSTelemetryConfig.cmake.in | Skips unofficial-sqlite3 dependency discovery when SQLite is bundled. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- lib/CMakeLists.txt: only add sqlite3_bundled to the install/export set when mat is a STATIC_LIBRARY. A shared mat absorbs the private SQLite into libmat and does not propagate the PRIVATE dependency, so exporting the separate archive there was unnecessary and could let a consumer link a second SQLite copy. For a static mat the archive must stay exported because the static library propagates its PRIVATE dependency through its link interface (\$<LINK_ONLY:...>). - CMakeLists.txt: make the vcpkg dependency-mode status message reflect whether the external sqlite3 package or the private minimal SQLite is used. Verified with an isolated CMake export test: static mat exports m+sq (consumer linking only the namespaced lib resolves sq); shared mat exports only m and install(EXPORT) succeeds with sq excluded. Re-ran the vcpkg [core,minimal-sqlite] consumer (static x64-linux): 10/10. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Correct the inline comment: a PRIVATE link of the bundled SQLite suppresses propagation of its include dirs / compile definitions, but a static mat still propagates the archive for linking via \$<LINK_ONLY:...> (hence it is exported for static builds); a shared mat absorbs it. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…efault-http-client feature Make the HTTP-client footprint a consumer choice instead of hardcoding curl[openssl]: - vcpkg.json: replace the base curl[openssl] dependency with three features -- curl-openssl (default; libcurl + OpenSSL), curl-mbedtls (libcurl + mbedTLS), and no-default-http-client (omit the built-in client). curl-openssl is a default feature so a plain install keeps current behavior; [core,no-default-http-client] drops curl entirely. - portfile.cmake: map the no-default-http-client feature to -DBUILD_CURL_HTTP_CLIENT=OFF via INVERTED_FEATURES. - CMakeLists.txt: when the built-in client is enabled in vcpkg mode but libcurl is not found, emit a clear FATAL_ERROR pointing at the curl-openssl/curl-mbedtls/ no-default-http-client features (instead of a bare find_package failure). - docs: document the size ladder (OpenSSL ~10.6MB / mbedTLS ~4.4MB / no-curl ~1.4MB) and the exact mbedTLS recipe -- crucially, the consumer must ALSO list curl with default-features:false at the top level, because vcpkg only honors curl's default-features:false for top-level dependencies (otherwise curl's ssl default pulls OpenSSL in transitively alongside mbedTLS). Validated on WSL with vcpkg: default resolves curl[openssl]+sqlite3; the documented mbedTLS recipe builds with mbedTLS only (no libssl/libcrypto, libcurl carries no OpenSSL symbols) and the consumer runs; [core,no-default-http-client] drops curl from the graph; [core,minimal-sqlite,no-default-http-client] drops curl and the external sqlite3. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ard (Copilot review) - CMakeLists.txt: in vcpkg mode use find_package(CURL CONFIG QUIET) and gate on TARGET CURL::libcurl. Forcing CONFIG selects the vcpkg-provided CURLConfig (which defines the imported target) rather than the module FindCURL, which on some CMake versions does not define CURL::libcurl and would fail at link. - portfile.cmake: fail fast when more than one of curl-openssl/curl-mbedtls/ no-default-http-client is selected. vcpkg cannot express mutual exclusivity, so a consumer requesting e.g. curl-mbedtls without [core] keeps the default curl-openssl and would union both TLS backends; the guard now errors with guidance to use the [core,...] form. Validated: the guard passes single selections and fires on curl-openssl+curl-mbedtls and curl-openssl+no-default-http-client; the default (curl-openssl) vcpkg consumer still configures via CURL CONFIG, links, and runs. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Drop the ability to build without the built-in libcurl HTTP client. That option only benefited consumers that already ship their own IHttpClient; the SDK's named consumers (and Apple/Windows, which use NSURLSession/WinInet) never needed it, and it added a fragile feature plus a config-flow opt-out. The TLS-backend selection (curl-openssl default / curl-mbedtls) and minimal-SQLite remain. - CMakeLists.txt: remove option(BUILD_CURL_HTTP_CLIENT) and the no-curl else branch; the curl HTTP client is always built on the CPP11/curl path again (keeping the find_package(CURL CONFIG) + TARGET CURL::libcurl hardening). - lib/include/mat/config.h: remove the MATSDK_NO_DEFAULT_HTTP_CLIENT -> HAVE_MAT_DEFAULT_HTTP_CLIENT opt-out. - vcpkg.json: remove the no-default-http-client feature. - portfile.cmake: remove the INVERTED_FEATURES mapping; the mutual-exclusivity guard now covers just curl-openssl vs curl-mbedtls. - docs: drop the no-curl row/section; note the size figures are worst-case (without consumer-side --gc-sections). Validated: vcpkg.json parses, CMake configures cleanly, and the mat target builds and links with the curl client compiled in. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Updated scope: removed the no-curl ( |
…kg packages
macOS/iOS ship libsqlite3 and libz as system libraries, so pulling and statically
linking the vcpkg sqlite3 + zlib packages added ~1 MB of redundant code to Apple
binaries. Link the system libraries instead -- consistent with the SDK's own Swift
Package (which links .linkedLibrary("sqlite3"/"z")) and with how analogous telemetry
SDKs (e.g. sentry-native) gate these deps off Apple platforms.
- vcpkg.json: gate the zlib dependency and the system-sqlite feature's sqlite3
dependency to "!osx & !ios" so they are not installed on Apple.
- CMakeLists.txt: on APPLE in vcpkg mode, find_package(SQLite3)/find_package(ZLIB)
(CMake's modules resolve to the OS libraries) and set MATSDK_APPLE_SYSTEM_DEPS.
- lib/CMakeLists.txt: link SQLite::SQLite3 + ZLIB::ZLIB on Apple; never bundle a
private SQLite on Apple (MATSDK_MINIMAL_SQLITE is a no-op there since the system
lib is already smaller).
- MSTelemetryConfig.cmake.in: re-find system SQLite3 on Apple, the vcpkg
unofficial-sqlite3 elsewhere.
- docs: note the Apple system-lib behavior.
Validated: non-Apple paths unchanged -- Linux vendored mat builds, and the Linux
vcpkg consumer's generated config resolves unofficial-sqlite3 (if(OFF)) and runs.
The Apple build itself needs validation on macOS/iOS CI (no Mac available here);
the risk is whether find_package(SQLite3) resolves the system lib under the vcpkg
Apple triplets' find-root settings.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Address Copilot review comment on lib/CMakeLists.txt:470. The comment claimed the build drops /WX for the vendored SQLite translation unit, but the code only added /w. /w disables all warnings, but MSVC can still promote a non-suppressible warning to an error under an inherited /WX. Add /WX- so the code literally matches the comment's stated intent and cannot be broken by such a warning. Verified cl.exe accepts /w /WX- together. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Address Copilot review comment on lib/CMakeLists.txt:446. Adding iOS to the legacy bundled-SQLite path means MATSDK_MINIMAL_SQLITE is no longer a strict no-op on all Apple builds: iOS in legacy mode (MATSDK_USE_VCPKG_DEPS=OFF) bundles the amalgamation and applies the strip definitions to it, matching Android legacy. Clarify the comment so it no longer reads as a blanket 'no effect on Apple' statement. No behavior change. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ion)
Take further inspiration from the ONNX Runtime consumer patch, verified against
what the repo already does on Apple.
The SDK's own iOS Xcode projects link libsqlite3.tbd + libz.tbd from the SDKROOT,
Package.swift links .linkedLibrary("sqlite3")/("z"), and microsoft#1499 already links the
system libsqlite3/libz on the vcpkg Apple path. Bundling is an Android-only
convention (the NDK ships no system zlib). So the earlier change that made iOS
legacy bundle sqlite+zlib was the inconsistent one; this aligns iOS with the rest
of the repo.
- Apple legacy (macOS + iOS) now links system `sqlite3 z` by portable names in a
single elseif(APPLE) branch. macOS moves off find_package(ZLIB) + hardcoded
Homebrew .a paths (non-relocatable) onto the same portable link names, so
exported static packages stay relocatable. iOS no longer bundles.
- iOS dropped from the MATSDK_BUNDLE_SQLITE gating and the bundled-zlib branch,
which are now Android-only.
- Exclude iOS from include_directories(/usr/local/include): that host (macOS) path
must not be injected into an iOS cross-compile's search path where it can shadow
the iOS SDK's own headers.
- Linux legacy simplified to find_package(SQLite3) (the Homebrew .a fallbacks were
macOS-only and are now handled by the Apple branch).
Verified: Linux top-level and add_subdirectory legacy builds both produce
libmat.so. The Apple legacy path is exercised by the macOS-latest CI leg
(build-posix-latest, legacy mode); iOS cannot be built on this host.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
target_include_directories(mat PRIVATE ${ZLIB_INCLUDE_DIRS}) was redundant: mat
already links ZLIB::ZLIB (and SQLite::SQLite3), imported targets that propagate
their own include directories. Verified: Linux legacy mat build still resolves
<zlib.h> and produces libmat.so.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…' into bhamehta/optional-curl-http-client
The port pinned v3.10.161.1 while the SDK source on this branch is at v3.10.173.1 (Version.hpp), leaving production installs two releases behind. Bump the portfile REF + SHA512 and the vcpkg.json version to v3.10.173.1; the SHA512 is computed from the release source tarball. Note: the port's minimal-sqlite and Apple system-sqlite features depend on CMake changes introduced by this PR that are not yet in any release tag. The in-repo port tests exercise them against local source via MATSDK_VCPKG_SOURCE_DIR, and the pinned REF must be advanced again to the release that includes these changes once it is cut. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
After updating the vcpkg port REF/SHA512/version for a new SDK release, exercise the real production port path with MATSDK_VCPKG_SOURCE_DIR unset. This catches mismatches where the port manifest assumes source changes that are not present in the release tag the port downloads. When the new footprint features are present, validate the opt-in minimal-sqlite + curl-openssl feature set so release automation covers both release pinning and feature wiring before opening the vcpkg PR. Validation: - Parsed .github/workflows/vcpkg-release-bump.yml with PyYAML. - Verified the feature-selection expression resolves to cpp-client-telemetry[core,minimal-sqlite,curl-openssl] for the current port. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Remove the release-bump production-port validation added in 93dd36e. The vcpkg port update will instead rely on the explicit release sequencing: merge the SDK source changes, cut a new SDK tag, then bump the vcpkg REF, SHA512, and version to that tag. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This reverts commit dd6007a.
Summary
Four changes that make the SDK cheaper to embed in a host application (e.g. ONNX Runtime / Foundry Local): three cut its binary footprint and dependency surface, and one folds in the downstream patch previously needed to consume it as a CMake subproject. All are opt-in or platform-scoped, so the default vcpkg install and default CMake build are unchanged (built-in curl HTTP client + external
sqlite3).1.
MATSDK_MINIMAL_SQLITE(default OFF) — private feature-stripped SQLiteThe SDK uses SQLite only for its offline event-storage cache (plain tables, transactions, WAL, autovacuum/
VACUUM, a few PRAGMAs, and one custom UTF-8 function).MATSDK_MINIMAL_SQLITE=ONcompiles a private SQLite from the vendored amalgamation with a set of amalgamation-safe strip flags (single source of truth:MATSDK_SQLITE_MINIMAL_DEFS), removing the externalsqlite3dependency and shrinking SQLite ~10.2% (.text) / ~12.5% (stripped object).minimal-sqlitefeature;sqlite3moves into a defaultsystem-sqlitefeature, so[core,minimal-sqlite]drops the external dependency.SQLITE_OMIT_AUTOINIT(theskipSqliteInitAndShutdownconfig lets the host skip the SDK's explicitsqlite3_initialize(), which the host cannot do against a private SQLite) andSQLITE_DEFAULT_MEMSTATUS=0(the SDK arms a soft heap limit viasqlite3_soft_heap_limit64()that is only enforced while memory statistics are on).matpropagates the private SQLite through its link interface, so it is exported/installed asMSTelemetry::sqlite3_bundled(static-only; a sharedmatabsorbs it). The footprint doc notes the static-absorption symbol-visibility caveat.2. Selectable curl TLS backend —
curl-openssl(default) /curl-mbedtlsOn Linux/Android the built-in HTTP client links libcurl, which pulls in a TLS backend. The port exposes the choice as two mutually-exclusive vcpkg features:
curl-openssl(default) —curl[core,openssl], unchanged behavior.curl-mbedtls—curl[core,mbedtls]for a smaller TLS footprint.Because vcpkg only honors
default-features: falsefor top-level dependencies, a mbedTLS-only build requires the[core,curl-mbedtls,...]form plus a top-levelcurl[core,mbedtls]entry in the consumer manifest (documented, with a--dry-run-verified recipe). On Linux/Android the port fails fast if both or neither curl backend is selected; these guards are scoped to Linux/Android, since Windows (WinInet) and Apple (NSURLSession) do not link curl.3. Apple: link the system
libsqlite3/libzon macOS/iOSmacOS/iOS ship
libsqlite3andlibz, so the port no longer pulls the vcpkgsqlite3/zlibpackages there (gated!osx & !iosin the manifest). The build usesfind_package(SQLite3)/find_package(ZLIB)(SQLite::SQLite3/ZLIB::ZLIB) — the same system libraries the SDK's Swift Package already links — removing ~1 MB of redundant bundled binaries on Apple.minimal-sqlitetherefore has no effect on Apple.4. Consume the SDK as a CMake subproject in legacy mode
Hosts that embed the SDK via
add_subdirectory()/FetchContentin legacy mode (MATSDK_USE_VCPKG_DEPS=OFF, e.g. ONNX Runtime / Foundry Local) previously had to carry a downstream patch. Two small changes fold that patch upstream:CMAKE_CURRENT_SOURCE_DIRinstead ofCMAKE_SOURCE_DIR, so the bundledsqlite/zlib/nlohmannheaders still resolve when this repo is not the top-level project.sqlite3andzby portable link names in a singleelseif(APPLE)branch. This matches what the repo already does on Apple — the iOS Xcode projects linklibsqlite3.tbd/libz.tbdfrom the SDKROOT,Package.swiftlinks.linkedLibrary("sqlite3")/("z"), and the vcpkg Apple path (section 3) links the same system libraries. macOS moves offfind_package(ZLIB)+ hardcoded Homebrew.apaths so exported static packages stay relocatable; nothing is bundled on Apple (bundling stays Android-only, where the NDK ships no system zlib).include_directories(/usr/local/include)so that host (macOS) path is not injected into an iOS cross-compile, where it can shadow the iOS SDK's own headers.Footprint impact
Validation
MATSDK_MINIMAL_SQLITE=ON[core,minimal-sqlite]consumerMSTelemetry::sqlite3_bundled, runs 10/10; external sqlite3 droppedsystem-sqlite+curl-openssl)curl-mbedtlsrecipevcpkg install --dry-run+ full build resolve tocurl[core,mbedtls], no OpenSSL built[core,minimal-sqlite]fails fast (no curl backend);[curl-mbedtls]w/o[core]fails (mutually exclusive); both messages give complete examplessqlite3_bundled+matcompile/link.text/ −12.5% stripped objectThe Apple system-libs path uses CMake's standard
FindSQLite3/FindZLIBand is validated by the macOS/iOS CI jobs (cannot be built locally without a Mac).Reviewed for correctness, security (the SQLite strip is security-neutral-to-positive —
SQLITE_DQS=0hardens identifier handling), and design; driven through the Copilot review loop.Co-authored-by: Copilot 223556219+Copilot@users.noreply.github.com