[RFC][Feature] Parsing Batterystats from Bugreports

[cphlipot1]

Background context

There is a power analysis tool called Battery Historian which allows loading bug reports and viewing various timeline stats and events like battery drain, wakeups, etc. However Battery Historian appears to have been deprecated and is now directing people to use alterate tools including Perfetto: https://developer.android.com/topic/performance/power/setup-battery-historian

Perfetto is currently capable of parsing battery stats data emitted by ATrace. It is also capable of parsing data from dumpstate in bug reports, but it does not appear to be capable of parsing Battery stats events from dumpstate contained within the bug reports.

Feature proposal

I wanted to propose closing this gap. It looks like there is already a logical place to batterystats parsing this alongside the existing android_log_reader which parses logcat from dumpstate: https://cs.android.com/android/platform/superproject/main/+/main:external/perfetto/src/trace_processor/importers/android_bugreport/android_dumpstate_reader.cc;l=117-119

In theory it should also be possible to insert these events in the SQLite DB in the same format as what results from importing battery stats data via atrace, so the existing Perfetto plugins should be able to operate transparently on the data regardless of the underlying file format.

I wanted to solicit comments on this:

• does the proposed feature make sense as part of perfetto or not?
• If yes, does the suggested implementation of extending the dumpstate importer make sense?

[LalitMaganti]

does the proposed feature make sense as part of perfetto or not?

Yes it absolutely makes sense to be part of Perfetto! The only reason it is not is because no one has done the work but otherwise, makes total sense!

If yes, does the suggested implementation of extending the dumpstate importer make sense?

Yes absolutely! When we wrote the bugreport importer, the idea was that we slowly extend the tool to support more of the data in bugreports over time but inside Google there is already a tool for ingesting bugreports and visualizing them so we did not feel like it was the best use of resources and the implementation did not get much further than just parsing logcat.

[cphlipot1]

I put up a CL to start on this:
https://android-review.googlesource.com/c/platform/external/perfetto/+/3368462

In theory it should also be possible to insert these events in the SQLite DB in the same format as what results from importing battery stats data via atrace

This CL only covers the data already emitted by atrace from BatteryStats.java.

I think there is additional data in the battery stats checkin that is still worth displaying (wake locks being one, screen state, and battery drain info being another). This data is available via atrace too, but doesn't come from battery stats, so if we want to import this data and have it used by existing plugins, we probably wouldn't be able to use the "battery_stats.*" tracks for everything like that current CL does.

[cphlipot1]

Thanks for all the reviews so far! i think we're parsing about 30-40% of the battery events at this point. I'm going to take a break on this work for a few weeks, but in the meantime I had a few questions.

1. I think the files test/data are stored on GCS so i'm not sure i can upload new files there. I think the current bugreport comes from an AOSP version that is quite old at this point. Something from Android 14 or newer would be nice to have here for diff tests as these newer OS builds contain events that are not present in the older ones. Would it it be possible to get a newer bugreport uploaded here?
2. Logcat i've noticed is currently out of sync with the battery stats events due to BatteryStats being in UTC while Logcat is in the local TZ. TZ offset info is not currently being parsed, so it is assumed to be zero, which is wrong most of the time. We can try to parse the TZ from the bugreport, but handing every timezone to compute the offet is a bit complex. C++ 20 includes timezone APIs that would be nice to be able to use here, but i understand Perfetto only supports up to C++ 17. Do you have plans to migrate to C++ 20 at some point, or would you be willing to accept limited use of C++ 20 APIs if we guard them with ifdefs?

[LalitMaganti]

Would it it be possible to get a newer bugreport uploaded here?

The GCS bucket is only writable by Googlers - if you give me a file, I can upload it for you.

Do you have plans to migrate to C++ 20 at some point, or would you be willing to accept limited use of C++ 20 APIs if we guard them with ifdefs?

I'm sympathetic to the complexity of timezones (https://www.zainrizvi.io/blog/falsehoods-programmers-believe-about-time-zones/) but unfortunately not: we cannot add C++20 features as TP has a bunch of embedders who are still 17 only. And I would really like to not ifdef and have diffferent features.

Is there no way to do it without 20?

[cphlipot1]

The GCS bucket is only writable by Googlers - if you give me a file, I can upload it for you.

Thanks for the offer, although if i had to provide one, i'm I'm not confident i'd be able to anonymize the bugreport sufficiently enough to feel comfortable uploading something that would be publicly available. Let me think on this a big more, but i'll probably keep using small synthetic datasets for the time being util i can think of an alternative.

Is there no way to do it without 20?

There are alternatives, just using 20 APIs seemed like the simplest and least risky options if it was available. third-party projects and writing our own are also alternatives here, although i'd really prefer to not write our own if possible.

We can also try to find two timestamps in the dumpstate file that were recorded while the dump was going on. So long as dumpstate didn't take too long to run, we should be able to diff the two timestamps and then round to the nearest valid tz_offset which is what we're really after.

So far i've found one candidate line which looks good. The alarm service outputs this line which could be leveraged:
" nowRTC=1629844744041=2021-08-24 23:39:04.041 nowELAPSED=403532"

The more i think about it, that line seems great for a variety of reasons. as not only does it give us what looks like the tz offset, but it gives us nowElapsed, wich i think maps to CLOCK_BOOTTIME (although i'd need to double check), so this could allow us to generate a fairly accurate clock snapshot as well.

The the main downsides I see are:

• We'd need to rely on the Alarm service to provide this line, and i'm not sure how appropriate it is to have such a critical dependency on the Alarm service dump in order for dumpstate to import properly.
• The alarm service doesn't appear in dumpstate until a significant way thorough the dumpstate file. At this point we'd have already parsed large number of logcat lines and sent them to the sorter already. Instead we'd probably need to buffer the parsed logcat events in memory and only send them to the sorter once we have a valid tz_offset, or instead maybe do multiple passes of the dumpstate file.

Let me know if you have any thoughts on this. I probably won't be able to work on anything here for a few week though.

[LalitMaganti]

Let me think on this a big more, but i'll probably keep using small synthetic datasets for the time being util i can think of an alternative.

Makes sense. One option is to flash a clean build with no account and then take a bugreport there. Afaik that should be pretty safe in terms of privacy.

We'd need to rely on the Alarm service to provide this line, and i'm not sure how appropriate it is to have such a critical dependency on the Alarm service dump in order for dumpstate to import properly.

I think it's ok to have this dependency. Correctness and simplicity is key and seems like both are achieved by using this line. I'd go for it.

The alarm service doesn't appear in dumpstate until a significant way thorough the dumpstate file. At this point we'd have already parsed large number of logcat lines and sent them to the sorter already. Instead we'd probably need to buffer the parsed logcat events in memory and only send them to the sorter once we have a valid tz_offset, or instead maybe do multiple passes of the dumpstate file.

Buffering like this is probably fine. At the end of the day, I've learned over time that people care more about things working than that performance is perfect. Yes it might be a few hundreds of MB in memory but it's better if the report is actually useful.

I probably won't be able to work on anything here for a few week though.

No worries, we can pick this up whenever you get time. Thanks so much for the contributions so far, it's been a pleasure working together!