RUM-14330: Optimizations to reduce .so file#3190
Conversation
ea86e35 to
b8c18c3
Compare
This comment has been minimized.
This comment has been minimized.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #3190 +/- ##
===========================================
- Coverage 71.38% 71.37% -0.01%
===========================================
Files 940 940
Lines 34773 34773
Branches 5894 5894
===========================================
- Hits 24822 24819 -3
+ Misses 8308 8306 -2
- Partials 1643 1648 +5 🚀 New features to boost your workflow:
|
89a5d6e to
9318ad7
Compare
0xnm
left a comment
0xnm
left a comment
There was a problem hiding this comment.
If everything works in Release build and NDK crash is captured and sent, then all good 👍
|
@codex review |
|
Codex Review: Didn't find any major issues. Bravo. ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
There was a problem hiding this comment.
@0xnm the compiler was optimizing and removing the code of sigsegv and sigill ndk crashes (even before my changes) in release builds, so the buttons on the sample app didn't work. Simplified this and confirmed NDK crashes are being sent to Datadog and working fine.
| #include <jni.h> | ||
| #include <signal.h> | ||
|
|
||
| void crash_with_sigsegv_signal() { |
There was a problem hiding this comment.
Can you explain the need for this change? raise indeed will raise the necessary signal, but the code here is to illustrate the real example leading to the particular signal.
There was a problem hiding this comment.
Rolled back to the previous code, just added some tweaks to prevent compiler optimization. Tested and crashes are working in sample app release build.
| -Werror -Wall -pedantic) | ||
|
|
||
| # Size optimizations for non-Debug builds | ||
| if(NOT CMAKE_BUILD_TYPE STREQUAL "Debug") |
There was a problem hiding this comment.
one thing to check for release builds: once NDK crash happens and if Datadog NDK makes a part of the stacktrace, can we still symbolicate such frames after binary shrinking?
There was a problem hiding this comment.
Thanks for pointing this out. First implementation had this problem, now fixed by removing fvisibility=hidden from compile options. Now, all function names are preserved and readable at runtime.
5c41110 to
46f748f
Compare
46f748f to
707ed9d
Compare
6f9c1f7 to
0a6bbcc
Compare
Co-Authored-By: Claude Opus 4.6 <[email protected]>
0a6bbcc to
704f62e
Compare
3 participants
What does this PR do?
.sofile size by around 50%.Motivation
The
.sofor arm64-v8a was ~860 KB — larger than the SDK's APK footprint (~570 KB) without thendk.Below are analysis of this file inside the Sample app release APK done in Android Studio, describing
Sizeon the left andDownload Sizeon the right.Before (sample app release APK on develop branch):
After (with these optimizations):
Review checklist (to be filled by reviewers)