-
Notifications
You must be signed in to change notification settings - Fork 91
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
Reduce overhead from using RaygunBreadcrumb by lazy allocation of CustomData #524
Conversation
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.
Thank you for the PR. I've reviewed it, generally happy with the changes, just missing braces on the if statement kicking off my OCD :)
Do have 1 question in regards to not using .ToList();
@@ -439,7 +439,7 @@ public void SendInBackground(Func<RaygunMessage> raygunMessage) | |||
private void StripAndSend(Exception exception, IList<string> tags, IDictionary userCustomData, RaygunIdentifierMessage userInfo, DateTime? currentTime) | |||
{ | |||
var requestMessage = BuildRequestMessage(); | |||
var breadcrumbs = _breadcrumbs.ToList(); | |||
IList<RaygunBreadcrumb> breadcrumbs = BuildBreadCrumbList(); |
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.
Curious to know why this is better than calling .ToList()
?
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.
_breadcrumbs.ToList()
will allocate an enumerator and a List
-object. The BuildBreadCrumbList()
will also allocate an enumerator, but only allocate a List
-object when breadcrumbs are found.
Level = RaygunBreadcrumbLevel.Info; | ||
Type = RaygunBreadcrumbType.Manual.ToString(); | ||
Type = nameof(RaygunBreadcrumbType.Manual); |
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.
Love this, I always forget to do it.
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.
Thank you for the PR!
Also skip StackTrace probing, when BreadCrumb is prefilled.