这是indexloc提供的服务，不要输入任何密码
Skip to content

fix: profile payload issues #2375

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
Merged
merged 11 commits into from
Nov 9, 2022
Merged

fix: profile payload issues #2375

merged 11 commits into from
Nov 9, 2022

Conversation

@armcknight
Copy link
Member

@armcknight armcknight commented Nov 9, 2022

There were a couple remaining issues with profile payloads:

  • sometimes we still try to send an envelope with 0 samples, presumably for very fast start/stop of profiler. we simply discard these and log for now
  • we weren't deduplicating frames across an entire profile, only per sample
  • we weren't performing a correct deep equality check between stacks

@armcknight
discard profiles with no samples in payload
a45a148
@armcknight
fix: frame and stack indexing
6d4bc53 
- keep frame index across entire profile, not just within one sample
- perform deep equality comparison for stacks instead of using indexOfObject:
@armcknight
ref: extract function to handle profile sampler backtrace callbacks
2cfd045
@armcknight
fix edge case for empty new stack
774b497
@armcknight
changelog
9a52046
@armcknight armcknight force-pushed the armcknight/fix/profile-payload-issues branch from 10f8753 to 9a52046 Compare November 9, 2022 16:47
indragiek
indragiek reviewed Nov 9, 2022
View reviewed changes
@armcknight armcknight changed the title Armcknight/fix/profile payload issues fix: profile payload issues Nov 9, 2022
@github-actions
Copy link
Contributor

github-actions bot commented Nov 9, 2022 •
edited
Loading

Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1235.98 ms 1243.10 ms 7.12 ms
Size 20.75 KiB 368.04 KiB 347.29 KiB

Baseline results on branch: master

Startup times

Revision Plain With Sentry Diff
5025d2e 1248.52 ms 1251.72 ms 3.20 ms
0fdf0b2 1243.92 ms 1250.86 ms 6.94 ms
e2f1150 1220.39 ms 1249.94 ms 29.55 ms
7c867f1 1206.16 ms 1234.88 ms 28.71 ms
02eeb15 1243.49 ms 1261.24 ms 17.76 ms
202334c 1265.41 ms 1277.34 ms 11.93 ms
58f558d 1216.15 ms 1246.22 ms 30.07 ms
4e037c4 1214.71 ms 1235.20 ms 20.49 ms
604586a 1216.06 ms 1249.10 ms 33.04 ms
075911d 1256.73 ms 1271.22 ms 14.49 ms

App size

Revision Plain With Sentry Diff
5025d2e 20.51 KiB 331.79 KiB 311.28 KiB
0fdf0b2 20.51 KiB 332.90 KiB 312.39 KiB
e2f1150 20.51 KiB 333.10 KiB 312.59 KiB
7c867f1 20.51 KiB 332.59 KiB 312.09 KiB
02eeb15 20.50 KiB 343.44 KiB 322.93 KiB
202334c 20.51 KiB 331.79 KiB 311.28 KiB
58f558d 20.50 KiB 338.99 KiB 318.49 KiB
4e037c4 20.50 KiB 361.79 KiB 341.29 KiB
604586a 20.51 KiB 333.15 KiB 312.65 KiB
075911d 20.51 KiB 332.90 KiB 312.39 KiB

Previous results on branch: armcknight/fix/profile-payload-issues

Startup times

Revision Plain With Sentry Diff
9727310 1225.33 ms 1249.63 ms 24.31 ms
1078baf 1219.42 ms 1256.00 ms 36.58 ms
fe417bb 1253.94 ms 1260.94 ms 7.00 ms
53cd1b8 1247.51 ms 1271.66 ms 24.15 ms

App size

Revision Plain With Sentry Diff
9727310 20.75 KiB 368.05 KiB 347.30 KiB
1078baf 20.75 KiB 368.11 KiB 347.36 KiB
fe417bb 20.75 KiB 368.05 KiB 347.29 KiB
53cd1b8 20.75 KiB 368.04 KiB 347.29 KiB

indragiek
indragiek reviewed Nov 9, 2022
View reviewed changes
indragiek
indragiek approved these changes Nov 9, 2022
View reviewed changes
@armcknight armcknight merged commit d4933ec into master Nov 9, 2022
@armcknight armcknight deleted the armcknight/fix/profile-payload-issues branch November 9, 2022 23:43
kevinrenskers added a commit that referenced this pull request Nov 14, 2022
@kevinrenskers
Merge branch 'master' into feat/1759-breadcrumbs-to-disk
6143773 
* master:
  Report pre-warmed app starts (#1969)
  fix: Too long flush duration (#2370)
  release: 7.30.2
  fix: profile payload issues (#2375)
kevinrenskers added a commit that referenced this pull request Nov 14, 2022
@kevinrenskers
Merge branch 'master' into feat/1777-no-session-update-for-dropped-ev…
8511692 
…ents

* master:
  feat: Store breadcrumbs to disk for OOM events (#2347)
  Report pre-warmed app starts (#1969)
  fix: Too long flush duration (#2370)
  release: 7.30.2
  fix: profile payload issues (#2375)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@indragiek indragiek indragiek approved these changes

@philipphofmann philipphofmann Awaiting requested review from philipphofmann

@brustolin brustolin Awaiting requested review from brustolin

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

@armcknight @indragiek