Sentry-java: Adding breadcrumbs taking too much on Android

[samiazar]

Recently I've decided to add breadcrumbs for getting more details on crash reports.
But I notice each time by using the addBreadcrumb() method it take too much time (about 10 ms) on the UI thread.

Here is how I try to measure the time of this method.

long time = System.currentTimeMillis();
Breadcrumb breadcrumb = new Breadcrumb();
breadcrumb.setCategory("Event");
breadcrumb.setMessage(s);
breadcrumb.setLevel(SentryLevel.INFO);
mScope.addBreadcrumb(breadcrumb);
Timber.w("Adding breadcrumbs take %s millis", System.currentTimeMillis() - time);

And here is the result:

.

I think it's not expected. For logging, I almost dropping a frame on the UI.
Do you have a plan to optimize this?
I really need this feature but now its cost is heavy

[bruno-garcia]

Do you have a beforeBreadcrumb callback defined in your SentryOptions?
Is scope synchronization enabled?

https://github.com/getsentry/sentry-java/blob/3ed2ff105dd246cbf0242e802543c710bcb14a0b/sentry/src/main/java/io/sentry/SentryOptions.java#L1106-L1108

What version of the SDK. Is this on a emulator, device? What device?

[samiazar]

Thank you for your quick response.

The sentry version was 3.0.0 and I didn't have a beforeBreadcrumb callback and scope synchronization was disabled.
But after more investigating finally, I figure it out that the problem is about using getCurrentDateTimeOrNull method from DateUtils class.
https://github.com/getsentry/sentry-java/blob/3ed2ff105dd246cbf0242e802543c710bcb14a0b/sentry/src/main/java/io/sentry/Breadcrumb.java#L64

This method takes too much time. I tried to use the constructor of the Breadcrumb that get Date object but this constructor was package-private.
After all, by updating the sentry version into 3.2.0 I see this constructor being public.
So I creating breadcrumb with a simple Date object. (just new Date())
My issue is resolved but maybe you want to optimize the getCurrentDateTimeOrNull for the future.

All these benchmarking had been tested on the Samsung-j7 prime.

[bruno-garcia]

@samiazar as you can see the linked PR above done to improve this.
We'll release a new preview tomorrow that'll include this fix.

[marandaneto]

@samiazar just be aware that new Date() will get device's timezone instead of UTC which is what we do with the getCurrentDateTimeOrNull.
the latest alpha has some improvements, please test it out and let us know if it got any better, thanks for reporting.

[samiazar]

Thank you guys for quick action.

I checked the latest alpha release and here is the result of my benchmark on the same device:

I can see improvement and the time for creating breadcrumb has optimized. Nice Work! 👌
But take look when I use new Breadcrumb(new Date()) how is the result:

Still, I prefer to create breadcrumbs in this way over the default constructor of it.
@marandaneto You are right; the ‍‍new Date() will get the device's timezone. But I think it's ok because I only want the correct sequence of breadcrumbs no matter in what time exactly they happened. So in the trade-off between performance and UTC, I will choose the performance.
Again, All these benchmarking had been tested on the Samsung-j7 prime.

[maciejwalkowiak]

@samiazar big thanks for reporting back the benchmark results!

[marandaneto]

@samiazar just out of curiosity, how did you measure your 2nd benchmarking?
I ask that because I see floating values and your code above subtract both longs, it can't be a float value unless you either did formatting or the code snippet is not accurate, thanks.

[samiazar]

@marandaneto for sure. I have used System.nanoTime() and then convert the nano time into millisecond.
The reason was to show more accurate spent time for evaluating.

[marandaneto]

@samiazar thanks, we've improved even more and 4.x is pretty fast, I've done some benchmarking with https://openjdk.java.net/projects/code-tools/jmh/ btw.