perf(instrumentation): Resolve SDK class availability at build time - #1375
perf(instrumentation): Resolve SDK class availability at build time#1375romtsn wants to merge 3 commits into
Conversation
Add an independently configurable ASM visitor that derives known optional classes from resolved modules and injects their availability into LoadClass. Avoid repeated reflection during SDK startup. Refs JAVA-654 Co-Authored-By: Codex <noreply@openai.com>
Document the build-time SDK class availability optimization introduced in #1375. Refs JAVA-654 Co-Authored-By: Codex <noreply@openai.com>
Use zero placeholders and document that visitMaxs triggers frame and max computation. Refs LINEAR-JAVA-654 Co-Authored-By: Codex <noreply@openai.com>
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Want reviews to match your repository better? Bugbot Learning can learn team-specific rules from PR activity. A team admin can enable Learning in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 94d96e4. Configure here.
| instrumentationContext.apiVersion.get(), | ||
| nextClassVisitor, | ||
| resolveClassAvailability(modules), | ||
| ) |
There was a problem hiding this comment.
Stale availability after dependency changes
High Severity
SdkOptimizationParameters only exposes the modules service as @Internal, so AGP's incremental ASM transform has no @Input fingerprint for the computed availability map. Availability is injected into LoadClass inside the Sentry jar; adding or removing a mapped dependency like Timber leaves that jar unchanged, so prior instrumentation can be reused and the SDK keeps trusting outdated true/false entries.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 94d96e4. Configure here.
| return LoadClassClassVisitor( | ||
| instrumentationContext.apiVersion.get(), | ||
| nextClassVisitor, | ||
| resolveClassAvailability(modules), |
There was a problem hiding this comment.
Variants share mutable module state
Medium Severity
All variants register and write into one SentryModulesService instance, then this visitor reads that shared mutable map while injecting availability. Parallel resolution of variant classpaths can overwrite another variant's modules, so debug-only dependencies such as Timber can be recorded as present or absent for the wrong variant.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 94d96e4. Configure here.
runningcode
left a comment
runningcode
left a comment
There was a problem hiding this comment.
Looks good!
One more thing, should we detect an incompatible version of the sentry-java sdk and then show an error?
| * We detect sentry-android SDK version using configurations.incoming.afterResolve. This is | ||
| * guaranteed to be executed BEFORE any of the build tasks/transforms are started. | ||
| * | ||
| * After detecting the sdk state, we use Gradle's shared build service to persist the state |
There was a problem hiding this comment.
good to delete this. this is false, build services are not kept across builds.
| import org.objectweb.asm.MethodVisitor | ||
| import org.objectweb.asm.Opcodes | ||
|
|
||
| internal class LoadClassClassVisitor( |
There was a problem hiding this comment.
I think a small javadoc on this one would be helpful
|
|
||
| open class SdkOptimizationExtension @Inject constructor(objects: ObjectFactory) { | ||
| /** Enables build-time optimizations of the Sentry SDK. Defaults to true. */ | ||
| val enabled: Property<Boolean> = objects.property(Boolean::class.java).convention(true) |
There was a problem hiding this comment.
aren't we going to have different optimizations? like properties, manifest and this one? Will they all be gated behind the same flag?
| import javax.inject.Inject | ||
| import org.gradle.api.model.ObjectFactory | ||
| import org.gradle.api.provider.Property | ||
|
|
There was a problem hiding this comment.
Should we name this StartupTimeOptimizations?
| import org.gradle.api.provider.Property | ||
|
|
||
| open class SdkOptimizationExtension @Inject constructor(objects: ObjectFactory) { | ||
| /** Enables build-time optimizations of the Sentry SDK. Defaults to true. */ |
There was a problem hiding this comment.
| /** Enables build-time optimizations of the Sentry SDK. Defaults to true. */ | |
| /** Enables run-time optimizations of the Sentry SDK at the cost of build time. Defaults to true. */ |
| * between builds and also during a single build, because transforms are run in parallel. | ||
| */ | ||
| val sentryModulesService = | ||
| val sdkOptimizationEnabled = extension.sdkOptimization.enabled.get() |
There was a problem hiding this comment.
like above, i think we should rename this to runtimeOptimizations or something similar
2 participants
Resolve the availability of optional Sentry SDK classes from the app's dependency graph and inject the results into
LoadClassduring bytecode instrumentation. The optimization is independent from tracing instrumentation, enabled by default, and can be disabled withsentry.sdkOptimization.enabled = false.The visitor safely ignores older SDK versions without the availability field. Applications using the supporting SDK avoid reflective availability probes during startup, while unknown class names still fall back to reflection.
Supporting SDK change: getsentry/sentry-java#5875
On a Pixel 2 XL, map construction and lookups were about 160× faster than the equivalent absent-heavy 11-class reflection batch. In the absent-heavy startup macrobenchmark, SDK initialization was 1.08% faster and full startup was 0.88% faster, though variance exceeded the measured delta. The representative 10-present/1-absent case showed no measurable improvement.
Verified with Spotless and focused visitor/plugin tests.
Refs JAVA-654