Skip to content

Chore: Only await futures#1310

Merged
marandaneto merged 4 commits into
getsentry:v7.0.0from
ueman:dont-await-non-futures
Mar 6, 2023
Merged

Chore: Only await futures#1310
marandaneto merged 4 commits into
getsentry:v7.0.0from
ueman:dont-await-non-futures

Conversation

@ueman

@ueman ueman commented Mar 2, 2023

Copy link
Copy Markdown
Collaborator

📜 Description

Fixes #870

By awaiting only futures, the performance can be marginally improved.
I did my best to find all instances, but I'm not sure I got all.

💡 Motivation and Context

My motivation is the strive for a better performance.

💚 How did you test it?

There are no changes in functionality, just in the implementation. So the existing tests cover these changes.

📝 Checklist

  • I reviewed submitted code
  • I added tests to verify changes
  • No new PII added or SDK only sends newly added PII if sendDefaultPii is enabled
  • I updated the docs if needed
  • All tests passing
  • No breaking changes

🔮 Next steps

@ueman ueman changed the title Don't await non futures Chore: Only await futures Mar 2, 2023
Comment thread CHANGELOG.md Outdated
@codecov

codecov Bot commented Mar 2, 2023

Copy link
Copy Markdown

Codecov Report

Patch coverage: 85.71% and project coverage change: +0.02 🎉

Comparison is base (009c313) 90.27% compared to head (373cc06) 90.30%.

Additional details and impacted files
@@            Coverage Diff             @@
##           v7.0.0    #1310      +/-   ##
==========================================
+ Coverage   90.27%   90.30%   +0.02%     
==========================================
  Files         164      164              
  Lines        5174     5189      +15     
==========================================
+ Hits         4671     4686      +15     
  Misses        503      503              
Impacted Files Coverage Δ
dart/lib/src/hub_adapter.dart 10.52% <0.00%> (ø)
dart/lib/src/sentry.dart 86.07% <50.00%> (+0.36%) ⬆️
dart/lib/src/hub.dart 81.17% <100.00%> (+0.57%) ⬆️
dart/lib/src/scope.dart 98.88% <100.00%> (+<0.01%) ⬆️
dart/lib/src/sentry_client.dart 96.89% <100.00%> (+0.05%) ⬆️
dart/lib/src/sentry_envelope_item.dart 97.87% <100.00%> (+0.14%) ⬆️
dart/lib/src/sentry_tracer.dart 95.33% <100.00%> (+0.03%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

Comment thread CHANGELOG.md
Co-authored-by: Manoel Aranda Neto <[email protected]>
@marandaneto

Copy link
Copy Markdown
Contributor

@ueman

I did my best to find all instances, but I'm not sure I got all.

I guess you covered most of them, maybe missing are BeforeSendCallback, BeforeSendTransactionCallback, searching for FutureOr<help as well.

Comment thread dart/lib/src/hub.dart
}

Future<Scope> _cloneAndRunWithScope(
FutureOr<Scope> _cloneAndRunWithScope(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Good catch

_CachedItem(this._dataFactory);

final Future<List<int>> Function() _dataFactory;
final FutureOr<List<int>> Function() _dataFactory;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Good catch here too

@marandaneto marandaneto left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thank you @ueman

@marandaneto
marandaneto merged commit 4218615 into getsentry:v7.0.0 Mar 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants