-
-
Notifications
You must be signed in to change notification settings - Fork 465
Fix misuses of CopyOnWriteArrayList
#4247
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.
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
Conversation
Performance metrics 🚀
|
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 9b49778 | 394.02 ms | 455.04 ms | 61.02 ms |
| 805cbe5 | 411.64 ms | 526.35 ms | 114.71 ms |
| ac1dee2 | 391.74 ms | 412.60 ms | 20.86 ms |
| beceaa2 | 410.46 ms | 451.02 ms | 40.56 ms |
| ed84ad9 | 386.57 ms | 392.58 ms | 6.02 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 9b49778 | 1.58 MiB | 2.21 MiB | 642.11 KiB |
| 805cbe5 | 1.58 MiB | 2.21 MiB | 642.07 KiB |
| ac1dee2 | 1.58 MiB | 2.21 MiB | 642.34 KiB |
| beceaa2 | 1.58 MiB | 2.21 MiB | 642.11 KiB |
| ed84ad9 | 1.58 MiB | 2.21 MiB | 642.35 KiB |
SentryTracer.childrenCopyOnWriteArrayList
|
Only found another occurrence where we are misusing |
acbfe6f to
04c2e16
Compare
a803296 to
71e9529
Compare
adinauer
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Co-authored-by: Alexander Dinauer <adinauer@users.noreply.github.com>
Instructions and example for changelogPlease add an entry to Example: ## Unreleased
- Fix misuses of `CopyOnWriteArrayList` ([#4247](https://github.com/getsentry/sentry-java/pull/4247))If none of the above apply, you can opt out of this check by adding |
📜 Description
CopyOnWriteArrayListallows us to access its elements without copying and without the possibility to cause aConcurrentModificationException.This PR modifies its usages so that we avoid copying in order to get better performance memory-wise.
We also make sure that we are using it correctly to avoid other issues (using the iterator to avoid a possible NPE that could happen with index-based accesses, and other cases where we do things that are not thread safe).
💡 Motivation and Context
Closes #2815
💚 How did you test it?
Tested as documented in #2815 (comment)
📝 Checklist
sendDefaultPIIis enabled.