perf: Replace java.net.URI with custom string parsing in Dsn#5448
Open
runningcode wants to merge 6 commits into
Open
perf: Replace java.net.URI with custom string parsing in Dsn#5448runningcode wants to merge 6 commits into
runningcode wants to merge 6 commits into
Conversation
📲 Install BuildsAndroid
|
Contributor
Performance metrics 🚀
|
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 62b579c | 312.88 ms | 361.57 ms | 48.70 ms |
| d501a7e | 307.33 ms | 341.94 ms | 34.61 ms |
| 2195398 | 345.88 ms | 411.71 ms | 65.82 ms |
| cf708bd | 434.73 ms | 502.96 ms | 68.22 ms |
| 27d7cf8 | 397.90 ms | 498.65 ms | 100.75 ms |
| 85d7417 | 347.21 ms | 394.35 ms | 47.15 ms |
| 5dee26b | 315.44 ms | 367.25 ms | 51.81 ms |
| 4c04bb8 | 307.93 ms | 362.34 ms | 54.41 ms |
| 5b1a06b | 310.56 ms | 362.79 ms | 52.22 ms |
| f634d01 | 359.58 ms | 433.88 ms | 74.30 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 62b579c | 0 B | 0 B | 0 B |
| d501a7e | 0 B | 0 B | 0 B |
| 2195398 | 0 B | 0 B | 0 B |
| cf708bd | 1.58 MiB | 2.11 MiB | 539.71 KiB |
| 27d7cf8 | 1.58 MiB | 2.12 MiB | 549.42 KiB |
| 85d7417 | 1.58 MiB | 2.10 MiB | 533.44 KiB |
| 5dee26b | 0 B | 0 B | 0 B |
| 4c04bb8 | 0 B | 0 B | 0 B |
| 5b1a06b | 0 B | 0 B | 0 B |
| f634d01 | 1.58 MiB | 2.10 MiB | 533.40 KiB |
Previous results on branch: no/custom-dsn-parser
Startup times
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 85d9e31 | 323.66 ms | 391.76 ms | 68.10 ms |
| 51a27f4 | 318.65 ms | 361.66 ms | 43.01 ms |
| 96d1d4a | 354.34 ms | 443.40 ms | 89.06 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 85d9e31 | 0 B | 0 B | 0 B |
| 51a27f4 | 0 B | 0 B | 0 B |
| 96d1d4a | 0 B | 0 B | 0 B |
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.
Reviewed by Cursor Bugbot for commit cacc249. Configure here.
0xadam-brown
approved these changes
May 20, 2026
Member
0xadam-brown
left a comment
There was a problem hiding this comment.
A few optionals; otherwise lgtm
The Dsn constructor used `new URI(dsnString).normalize()` to parse the DSN string, which is known to be slow on Android. Since `retrieveParsedDsn()` is called on the main thread during `Sentry.init()` via `preInitConfigurations()`, this directly impacts app startup time. Replace the URI-based parsing with manual indexOf/substring operations. The only remaining URI construction is from pre-parsed components (`new URI(scheme, null, host, port, path, null, null)`), which is significantly cheaper since the JDK doesn't need to re-parse a string. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Cover edge cases specific to the manual indexOf/substring parser: null input, missing scheme separator, no slash after host, multiple path segments, port with path, multiple double slashes, query string with port, empty secret key, and a realistic Sentry DSN with org id. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Harden the custom DSN parser and convert its tests to Google Truth. - Strip URI fragments (#...) alongside query strings so they no longer leak into the project id and corrupt the constructed Sentry URI. - Detect bracketed IPv6 literal hosts when locating the port separator, restoring behavior that java.net.URI handled. - Narrow the parse error handling from catch (Throwable) to the expected exceptions, which stops swallowing Error and removes the doubled exception message. - Extract the parsing steps into focused private helpers. - Convert DsnTest to Google Truth assertions. - Move the changelog entry to the Unreleased section, since 8.43.0 and 8.43.1 have already been released. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
cacc249 to
4851e6d
Compare
Follow Truth's recommended pattern for exception testing: catch with assertFailsWith, then assert on the caught throwable with assertThat(ex).hasMessageThat(). Also assert the message in the previously bare throw-only cases so they can no longer pass on an unrelated exception. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Parse the port in a dedicated helper that reports the offending value
("Invalid DSN: Invalid port 'abc'.") instead of leaking the raw
NumberFormatException text. Narrow the catch to URISyntaxException now
that the port is the only parseInt, and add a test.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
0xadam-brown
approved these changes
Jun 4, 2026
Member
0xadam-brown
left a comment
There was a problem hiding this comment.
Thanks for the updates – lgtm 👍
| camerax-lifecycle = { module = "androidx.camera:camera-lifecycle", version.ref = "camerax" } | ||
| camerax-view = { module = "androidx.camera:camera-view", version.ref = "camerax" } | ||
|
|
||
| google-truth = { module = "com.google.truth:truth", version = "1.4.5" } |
Member
There was a problem hiding this comment.
l: No strong opinions about introducing a new assertion library for tests + it's helpful for verifying error msgs, so I'm okay with this even though we'll only have a few tests using Truth.
Anyone else can feel free to chime in if they think otherwise...
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

Summary
Replaces
java.net.URIparsing in theDsnconstructor with lightweight manualstring parsing, and hardens and tests the parser. This is a critical startup path performance improvement.
Here are perfetto traces of the
retrieveParsedDsn()method as measured on a Pixel 3. I've repeated the measurement several times and I always get an order of magnitude difference.This is on master:


This is on this branch:
What changed
new URI(dsnString).normalize()with manualindexOf/substringparsing, avoiding thejava.net.URIstring parser.retrieveParsedDsn()runs on the main thread duringSentry.init(), so thiskeeps a known-slow API off the init path. The only remaining
URIuse isconstructing the endpoint from already-parsed components
(
new URI(scheme, null, host, port, path, null, null)), which does notre-parse a string.
#…) alongside query strings so they nolonger leak into the project id, and support bracketed IPv6 literal hosts
(e.g.
https://key@[::1]:9000/1) — both behaviors the oldURIparser handled.JAVA-532
Relates to: https://linear.app/getsentry/issue/JAVA-532
Also relates to: https://linear.app/getsentry/issue/JAVA-516/move-dsn-parsing-off-the-main-thread-during-init
🤖 Generated with Claude Code