Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat(core): Further optimize debug ID parsing #14365

Merged
merged 8 commits into from
Nov 27, 2024
Merged

feat(core): Further optimize debug ID parsing #14365

merged 8 commits into from
Nov 27, 2024

Conversation

timfish
Copy link
Collaborator

@timfish timfish commented Nov 19, 2024 •
edited
Loading

This PR further optimises debug ID parsing.

  • Removes some side-effects. We only init the Map if we're actually using debug IDs
  • If globalThis._sentryDebugIds hasn't been modified since the last call, we return a cached result
  • Caches { filename, debugId } rather than StackFrame[] so we no longer walk the frames every time
  • Changes ANR to simply call getFilenameToDebugIdMap() since we can now rely on the caching
  • I removed the cache per stack parser because we don't use multiple stack parsers and this saved most of the addition to the bundle size

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Nov 19, 2024 •
edited
Loading

size-limit report 📦

Path Size % Change Change
@sentry/browser 23.12 KB +0.11% +25 B 🔺
@sentry/browser - with treeshaking flags 21.84 KB +0.12% +26 B 🔺
@sentry/browser (incl. Tracing) 35.7 KB +0.05% +17 B 🔺
@sentry/browser (incl. Tracing, Replay) 72.6 KB +0.05% +37 B 🔺
@sentry/browser (incl. Tracing, Replay) - with treeshaking flags 62.97 KB -0.01% -2 B 🔽
@sentry/browser (incl. Tracing, Replay with Canvas) 76.92 KB +0.05% +33 B 🔺
@sentry/browser (incl. Tracing, Replay, Feedback) 89.37 KB +0.03% +25 B 🔺
@sentry/browser (incl. Feedback) 39.86 KB +0.09% +34 B 🔺
@sentry/browser (incl. sendFeedback) 27.73 KB +0.04% +9 B 🔺
@sentry/browser (incl. FeedbackAsync) 32.53 KB +0.08% +26 B 🔺
@sentry/react 25.8 KB +0.09% +23 B 🔺
@sentry/react (incl. Tracing) 38.51 KB +0.05% +16 B 🔺
@sentry/vue 27.28 KB +0.03% +7 B 🔺
@sentry/vue (incl. Tracing) 37.49 KB +0.02% +4 B 🔺
@sentry/svelte 23.27 KB +0.09% +21 B 🔺
CDN Bundle 24.29 KB +0.06% +14 B 🔺
CDN Bundle (incl. Tracing) 37.31 KB +0.04% +13 B 🔺
CDN Bundle (incl. Tracing, Replay) 72.21 KB +0.03% +15 B 🔺
CDN Bundle (incl. Tracing, Replay, Feedback) 77.55 KB +0.03% +17 B 🔺
CDN Bundle - uncompressed 71.37 KB +0.03% +20 B 🔺
CDN Bundle (incl. Tracing) - uncompressed 110.69 KB +0.02% +20 B 🔺
CDN Bundle (incl. Tracing, Replay) - uncompressed 223.76 KB +0.01% +20 B 🔺
CDN Bundle (incl. Tracing, Replay, Feedback) - uncompressed 236.97 KB +0.01% +20 B 🔺
@sentry/nextjs (client) 38.83 KB +0.01% +1 B 🔺
@sentry/sveltekit (client) 36.21 KB +0.07% +24 B 🔺
@sentry/node 134.8 KB +0.02% +15 B 🔺
@sentry/node - without tracing 96.64 KB +0.03% +24 B 🔺
@sentry/aws-serverless 106.85 KB +0.02% +19 B 🔺

View base workflow run

@timfish
Copy link
Collaborator Author

timfish commented Nov 19, 2024

I'm disappointed this hits the bundle size like this 😭

Let me see if I can help that...

@timfish timfish marked this pull request as ready for review November 20, 2024 03:46
@timfish timfish requested a review from lforst November 20, 2024 14:58
AbhiPrasad
AbhiPrasad approved these changes Nov 21, 2024
View reviewed changes
Copy link
Member

@AbhiPrasad AbhiPrasad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bundle size reduction!

@lforst
Copy link
Member

lforst commented Nov 26, 2024

Since the size-checker doesn't seem entirely happy with this PR I would suggest keeping the logic for multiple stack parsers. I know it is unlikely to happen that you have more than 1 but it is technically correct.

The rest seems good though 👍

@timfish
Copy link
Collaborator Author

timfish commented Nov 26, 2024 •
edited
Loading

Before I removed the logic for multiple stack parsers, the size additions were 2-3x what they are now.

@lforst
Copy link
Member

lforst commented Nov 27, 2024

Before I removed the logic for multiple stack parsers, the size additions were 2-3x what they are now.

convinced

@lforst lforst merged commit de65590 into develop Nov 27, 2024
152 checks passed
@lforst lforst deleted the timfish/feat/debug-id-parse branch November 27, 2024 08:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AbhiPrasad AbhiPrasad AbhiPrasad approved these changes

@lforst lforst lforst approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@timfish @lforst @AbhiPrasad