In-memory-only SummaryEventStore

[ogolberg]

TL;DR; SharedPrefsSummaryEventStore sometimes causes performance issues. Would you consider providing an in-memory only implementation and allowing to use it via a configuration option?

Is your feature request related to a problem? Please describe.

Our app uses Launchdarkly extensively (many feature flags, evaluated often). We noticed a lot of ANRs on lower end devices caused by SharedPrefsSummaryEventStore. The ANRs mostly fall into two categories: serialization of the summary data structure when called from the main thread and the activity lifecycle waiting for SharedPreferences' fsync to complete (a known, unfortunate side effect of using SharedPreferences).

Describe the solution you'd like

We've implemented an in-memory SummaryEventStore, which looks exactly like SharedPrefsSummaryEventStore but backed by a simple map instead of SharedPreferences and observed a huge improvement. Of course, the tradeoff here is durability of events - if the app restarts, unflushed event summary is lost; for us, the performance benefits are definitely worth it.

Happy to open a PR with our patch.

Describe alternatives you've considered

It's possible to improve durability in offline scenarios by periodically snapshotting the in-memory event summary to disk in the background. This will not make much of a difference when the device is online, because the summary is periodically flushed back to the LD webservice anyway, and may not be worth the added complexity.

[louis-launchdarkly]

Hello @ogolberg, thank you for recommending the feature request. This is an issue we noticed too that the Android SDK right now is doing a lot of extra reading from the SharedPrefsSummaryEventStore.

The next major version of this SDK will have a fix for this, and I am marking this as a feature enhancement.

[eli-darkly]

Yeah— we flagged this internally as a concern a while back, and it seemed like it was probably just a holdover from some early implementation work where one of the original SDK developers had seen "we're keeping some state in SharedPreferences" and wrongly assumed that the summary event data should also be in that category. We had held off on changing it because (1) other Android improvements took priority and (2) I wasn't 100% sure that there wasn't some Android-specific reason why we had to make sure that that data could outlive the application.

But by now I think it's clear that the answer to (2) is no. The performance issues greatly outweigh the chance that an application will die before having a chance to send the summary event, and in the latter case, other event data would be lost anyway and the summary data isn't uniquely significant.

As Louis said, the upcoming 4.0.0 release will include this change. Since 4.0.0 will also include other API changes that people might not be ready to adopt immediately, and we're planning to release another 3.x minor version anyway as a transitional step, we may also want to do a similar fix in 3.x. But I think we'll have to wait till we're closer to being finished with the 4.0.0 work. (filed this internally as 176628)

Another inefficiency in our current use of SharedPreferences is that the SDK is re-querying the flag data from SharedPreferences and re-parsing it from JSON repeatedly, even when no flags have changed. That part will also be fixed in 4.0.0 but is less likely to be addressed in a 3.x release, because of architectural differences (i.e. in the older SDK code, that logic was not encapsulated in a convenient way). But I think that is not nearly as big a performance problem as writing to SharedPreferences is.

[ogolberg]

Another inefficiency in our current use of SharedPreferences is that the SDK is re-querying the flag data from SharedPreferences and re-parsing it from JSON repeatedly

Yes! Once we patched SummaryEventStore internally, we noticed (a much smaller number of) ANRs caused by Flag deserialization when flags are evaluated on the main thread.

[eli-darkly]

This change has been made in the 3.2.3 release.

[eli-darkly]

That is, we have switched to keeping the SummaryEventStore data in a regular in-memory data structure and not writing it to SharedPreferences. The other issue I mentioned ("the SDK is re-querying the flag data from SharedPreferences and re-parsing it from JSON repeatedly") is not changed in this release.