From 73597dcb9a737d5e49b98d38bb6952457de491a2 Mon Sep 17 00:00:00 2001 From: Tej Singh Date: Fri, 13 Mar 2020 18:42:40 -0700 Subject: [PATCH] Statsd update for native puller api feedback Update statsd to take in times in milliseconds instead of nanoseconds. Also make appropriate updates for graphics stats, odpm, subsystem sleep state, and LibStatsPullTests Test: atest LibStatsPullTests Test: bit statsd_test:* Bug: 150788562 Change-Id: I593552d6c50bb4dcb89ca9cc1c737781653e7cc5 --- apex/statsd/aidl/android/os/IStatsd.aidl | 2 +- .../libstatspull/jni/stats_pull_helper.cpp | 27 ++++++------- .../os/statsd/libstats/LibStatsPullTests.java | 38 +++++++++---------- cmds/statsd/src/StatsService.cpp | 10 +++-- cmds/statsd/src/StatsService.h | 5 ++- libs/hwui/jni/GraphicsStatsService.cpp | 10 ++--- ...server_stats_pull_StatsPullAtomService.cpp | 14 +++---- 7 files changed, 52 insertions(+), 54 deletions(-) diff --git a/apex/statsd/aidl/android/os/IStatsd.aidl b/apex/statsd/aidl/android/os/IStatsd.aidl index 445ae1d7a13eb..d5b5949cd0320 100644 --- a/apex/statsd/aidl/android/os/IStatsd.aidl +++ b/apex/statsd/aidl/android/os/IStatsd.aidl @@ -196,7 +196,7 @@ interface IStatsd { * * Enforces the REGISTER_STATS_PULL_ATOM permission. */ - oneway void registerNativePullAtomCallback(int atomTag, long coolDownNs, long timeoutNs, + oneway void registerNativePullAtomCallback(int atomTag, long coolDownMillis, long timeoutMillis, in int[] additiveFields, IPullAtomCallback pullerCallback); /** diff --git a/apex/statsd/tests/libstatspull/jni/stats_pull_helper.cpp b/apex/statsd/tests/libstatspull/jni/stats_pull_helper.cpp index 9e5aa952d9bc3..166592d351517 100644 --- a/apex/statsd/tests/libstatspull/jni/stats_pull_helper.cpp +++ b/apex/statsd/tests/libstatspull/jni/stats_pull_helper.cpp @@ -44,30 +44,27 @@ static AStatsManager_PullAtomCallbackReturn pullAtomCallback(int32_t atomTag, AS return sPullReturnVal; } -extern "C" -JNIEXPORT void JNICALL -Java_com_android_internal_os_statsd_libstats_LibStatsPullTests_registerStatsPuller( - JNIEnv* /*env*/, jobject /* this */, jint atomTag, jlong timeoutNs, jlong coolDownNs, - jint pullRetVal, jlong latencyMillis, int atomsPerPull) -{ +extern "C" JNIEXPORT void JNICALL +Java_com_android_internal_os_statsd_libstats_LibStatsPullTests_setStatsPuller( + JNIEnv* /*env*/, jobject /* this */, jint atomTag, jlong timeoutMillis, + jlong coolDownMillis, jint pullRetVal, jlong latencyMillis, int atomsPerPull) { sAtomTag = atomTag; sPullReturnVal = pullRetVal; sLatencyMillis = latencyMillis; sAtomsPerPull = atomsPerPull; sNumPulls = 0; AStatsManager_PullAtomMetadata* metadata = AStatsManager_PullAtomMetadata_obtain(); - AStatsManager_PullAtomMetadata_setCoolDownNs(metadata, coolDownNs); - AStatsManager_PullAtomMetadata_setTimeoutNs(metadata, timeoutNs); + AStatsManager_PullAtomMetadata_setCoolDownMillis(metadata, coolDownMillis); + AStatsManager_PullAtomMetadata_setTimeoutMillis(metadata, timeoutMillis); - AStatsManager_registerPullAtomCallback(sAtomTag, &pullAtomCallback, metadata, nullptr); + AStatsManager_setPullAtomCallback(sAtomTag, metadata, &pullAtomCallback, nullptr); AStatsManager_PullAtomMetadata_release(metadata); } -extern "C" -JNIEXPORT void JNICALL -Java_com_android_internal_os_statsd_libstats_LibStatsPullTests_unregisterStatsPuller( - JNIEnv* /*env*/, jobject /* this */, jint /*atomTag*/) -{ - AStatsManager_unregisterPullAtomCallback(sAtomTag); +extern "C" JNIEXPORT void JNICALL +Java_com_android_internal_os_statsd_libstats_LibStatsPullTests_clearStatsPuller(JNIEnv* /*env*/, + jobject /* this */, + jint /*atomTag*/) { + AStatsManager_clearPullAtomCallback(sAtomTag); } } // namespace diff --git a/apex/statsd/tests/libstatspull/src/com/android/internal/os/statsd/libstats/LibStatsPullTests.java b/apex/statsd/tests/libstatspull/src/com/android/internal/os/statsd/libstats/LibStatsPullTests.java index d4e51e040d1c5..3f199e83ceed9 100644 --- a/apex/statsd/tests/libstatspull/src/com/android/internal/os/statsd/libstats/LibStatsPullTests.java +++ b/apex/statsd/tests/libstatspull/src/com/android/internal/os/statsd/libstats/LibStatsPullTests.java @@ -58,8 +58,8 @@ public class LibStatsPullTests { private static int sPullReturnValue; private static long sConfigId; private static long sPullLatencyMillis; - private static long sPullTimeoutNs; - private static long sCoolDownNs; + private static long sPullTimeoutMillis; + private static long sCoolDownMillis; private static int sAtomsPerPull; static { @@ -75,8 +75,8 @@ public class LibStatsPullTests { assertThat(InstrumentationRegistry.getInstrumentation()).isNotNull(); sPullReturnValue = StatsManager.PULL_SUCCESS; sPullLatencyMillis = 0; - sPullTimeoutNs = 10_000_000_000L; - sCoolDownNs = 1_000_000_000L; + sPullTimeoutMillis = 10_000L; + sCoolDownMillis = 1_000L; sAtomsPerPull = 1; } @@ -85,7 +85,7 @@ public class LibStatsPullTests { */ @After public void tearDown() throws Exception { - unregisterStatsPuller(PULL_ATOM_TAG); + clearStatsPuller(PULL_ATOM_TAG); StatsManager statsManager = (StatsManager) mContext.getSystemService( Context.STATS_MANAGER); statsManager.removeConfig(sConfigId); @@ -102,14 +102,14 @@ public class LibStatsPullTests { createAndAddConfigToStatsd(statsManager); // Add the puller. - registerStatsPuller(PULL_ATOM_TAG, sPullTimeoutNs, sCoolDownNs, sPullReturnValue, + setStatsPuller(PULL_ATOM_TAG, sPullTimeoutMillis, sCoolDownMillis, sPullReturnValue, sPullLatencyMillis, sAtomsPerPull); Thread.sleep(SHORT_SLEEP_MILLIS); StatsLog.logStart(APP_BREADCRUMB_LABEL); // Let the current bucket finish. Thread.sleep(LONG_SLEEP_MILLIS); List data = StatsConfigUtils.getGaugeMetricDataList(statsManager, sConfigId); - unregisterStatsPuller(PULL_ATOM_TAG); + clearStatsPuller(PULL_ATOM_TAG); assertThat(data.size()).isEqualTo(1); TestAtoms.PullCallbackAtomWrapper atomWrapper = null; try { @@ -135,14 +135,14 @@ public class LibStatsPullTests { createAndAddConfigToStatsd(statsManager); sPullReturnValue = StatsManager.PULL_SKIP; // Add the puller. - registerStatsPuller(PULL_ATOM_TAG, sPullTimeoutNs, sCoolDownNs, sPullReturnValue, + setStatsPuller(PULL_ATOM_TAG, sPullTimeoutMillis, sCoolDownMillis, sPullReturnValue, sPullLatencyMillis, sAtomsPerPull); Thread.sleep(SHORT_SLEEP_MILLIS); StatsLog.logStart(APP_BREADCRUMB_LABEL); // Let the current bucket finish. Thread.sleep(LONG_SLEEP_MILLIS); List data = StatsConfigUtils.getGaugeMetricDataList(statsManager, sConfigId); - unregisterStatsPuller(PULL_ATOM_TAG); + clearStatsPuller(PULL_ATOM_TAG); assertThat(data.size()).isEqualTo(0); } @@ -157,17 +157,17 @@ public class LibStatsPullTests { // The puller will sleep for 1.5 sec. sPullLatencyMillis = 1_500; // 1 second timeout - sPullTimeoutNs = 1_000_000_000; + sPullTimeoutMillis = 1_000; // Add the puller. - registerStatsPuller(PULL_ATOM_TAG, sPullTimeoutNs, sCoolDownNs, sPullReturnValue, + setStatsPuller(PULL_ATOM_TAG, sPullTimeoutMillis, sCoolDownMillis, sPullReturnValue, sPullLatencyMillis, sAtomsPerPull); Thread.sleep(SHORT_SLEEP_MILLIS); StatsLog.logStart(APP_BREADCRUMB_LABEL); // Let the current bucket finish and the pull timeout. Thread.sleep(sPullLatencyMillis * 2); List data = StatsConfigUtils.getGaugeMetricDataList(statsManager, sConfigId); - unregisterStatsPuller(PULL_ATOM_TAG); + clearStatsPuller(PULL_ATOM_TAG); assertThat(data.size()).isEqualTo(0); } @@ -181,9 +181,9 @@ public class LibStatsPullTests { createAndAddConfigToStatsd(statsManager); // Set the cooldown to 10 seconds - sCoolDownNs = 10_000_000_000L; + sCoolDownMillis = 10_000L; // Add the puller. - registerStatsPuller(PULL_ATOM_TAG, sPullTimeoutNs, sCoolDownNs, sPullReturnValue, + setStatsPuller(PULL_ATOM_TAG, sPullTimeoutMillis, sCoolDownMillis, sPullReturnValue, sPullLatencyMillis, sAtomsPerPull); Thread.sleep(SHORT_SLEEP_MILLIS); @@ -192,7 +192,7 @@ public class LibStatsPullTests { StatsLog.logStart(APP_BREADCRUMB_LABEL); Thread.sleep(LONG_SLEEP_MILLIS); List data = StatsConfigUtils.getGaugeMetricDataList(statsManager, sConfigId); - unregisterStatsPuller(PULL_ATOM_TAG); + clearStatsPuller(PULL_ATOM_TAG); assertThat(data.size()).isEqualTo(2); for (int i = 0; i < data.size(); i++) { TestAtoms.PullCallbackAtomWrapper atomWrapper = null; @@ -221,7 +221,7 @@ public class LibStatsPullTests { createAndAddConfigToStatsd(statsManager); sAtomsPerPull = 1000; // Add the puller. - registerStatsPuller(PULL_ATOM_TAG, sPullTimeoutNs, sCoolDownNs, sPullReturnValue, + setStatsPuller(PULL_ATOM_TAG, sPullTimeoutMillis, sCoolDownMillis, sPullReturnValue, sPullLatencyMillis, sAtomsPerPull); Thread.sleep(SHORT_SLEEP_MILLIS); @@ -229,7 +229,7 @@ public class LibStatsPullTests { // Let the current bucket finish. Thread.sleep(LONG_SLEEP_MILLIS); List data = StatsConfigUtils.getGaugeMetricDataList(statsManager, sConfigId); - unregisterStatsPuller(PULL_ATOM_TAG); + clearStatsPuller(PULL_ATOM_TAG); assertThat(data.size()).isEqualTo(sAtomsPerPull); for (int i = 0; i < data.size(); i++) { @@ -276,9 +276,9 @@ public class LibStatsPullTests { assertThat(StatsConfigUtils.verifyValidConfigExists(statsManager, sConfigId)).isTrue(); } - private native void registerStatsPuller(int atomTag, long timeoutNs, long coolDownNs, + private native void setStatsPuller(int atomTag, long timeoutMillis, long coolDownMillis, int pullReturnVal, long latencyMillis, int atomPerPull); - private native void unregisterStatsPuller(int atomTag); + private native void clearStatsPuller(int atomTag); } diff --git a/cmds/statsd/src/StatsService.cpp b/cmds/statsd/src/StatsService.cpp index 7ad982671860b..812d10be4bf6b 100644 --- a/cmds/statsd/src/StatsService.cpp +++ b/cmds/statsd/src/StatsService.cpp @@ -1209,9 +1209,10 @@ Status StatsService::registerPullAtomCallback(int32_t uid, int32_t atomTag, int6 return Status::ok(); } -Status StatsService::registerNativePullAtomCallback(int32_t atomTag, int64_t coolDownNs, - int64_t timeoutNs, const std::vector& additiveFields, - const shared_ptr& pullerCallback) { +Status StatsService::registerNativePullAtomCallback( + int32_t atomTag, int64_t coolDownMillis, int64_t timeoutMillis, + const std::vector& additiveFields, + const shared_ptr& pullerCallback) { if (!checkPermission(kPermissionRegisterPullAtom)) { return exception( EX_SECURITY, @@ -1220,7 +1221,8 @@ Status StatsService::registerNativePullAtomCallback(int32_t atomTag, int64_t coo } VLOG("StatsService::registerNativePullAtomCallback called."); int32_t uid = AIBinder_getCallingUid(); - mPullerManager->RegisterPullAtomCallback(uid, atomTag, coolDownNs, timeoutNs, additiveFields, + mPullerManager->RegisterPullAtomCallback(uid, atomTag, MillisToNano(coolDownMillis), + MillisToNano(timeoutMillis), additiveFields, pullerCallback); return Status::ok(); } diff --git a/cmds/statsd/src/StatsService.h b/cmds/statsd/src/StatsService.h index c256e1f2ae9c1..114c84f953c87 100644 --- a/cmds/statsd/src/StatsService.h +++ b/cmds/statsd/src/StatsService.h @@ -175,8 +175,9 @@ public: /** * Binder call to register a callback function for a pulled atom. */ - virtual Status registerNativePullAtomCallback(int32_t atomTag, int64_t coolDownNs, - int64_t timeoutNs, const std::vector& additiveFields, + virtual Status registerNativePullAtomCallback( + int32_t atomTag, int64_t coolDownMillis, int64_t timeoutMillis, + const std::vector& additiveFields, const shared_ptr& pullerCallback) override; /** diff --git a/libs/hwui/jni/GraphicsStatsService.cpp b/libs/hwui/jni/GraphicsStatsService.cpp index 6076552fc094d..1591ffabd26ac 100644 --- a/libs/hwui/jni/GraphicsStatsService.cpp +++ b/libs/hwui/jni/GraphicsStatsService.cpp @@ -158,17 +158,17 @@ static AStatsManager_PullAtomCallbackReturn graphicsStatsPullCallback(int32_t at static void nativeInit(JNIEnv* env, jobject javaObject) { gGraphicsStatsServiceObject = env->NewGlobalRef(javaObject); AStatsManager_PullAtomMetadata* metadata = AStatsManager_PullAtomMetadata_obtain(); - AStatsManager_PullAtomMetadata_setCoolDownNs(metadata, 10 * 1000000); // 10 milliseconds - AStatsManager_PullAtomMetadata_setTimeoutNs(metadata, 2 * NS_PER_SEC); // 2 seconds + AStatsManager_PullAtomMetadata_setCoolDownMillis(metadata, 10); // 10 milliseconds + AStatsManager_PullAtomMetadata_setTimeoutMillis(metadata, 2 * MS_PER_SEC); // 2 seconds - AStatsManager_registerPullAtomCallback(android::util::GRAPHICS_STATS, - &graphicsStatsPullCallback, metadata, nullptr); + AStatsManager_setPullAtomCallback(android::util::GRAPHICS_STATS, metadata, + &graphicsStatsPullCallback, nullptr); AStatsManager_PullAtomMetadata_release(metadata); } static void nativeDestructor(JNIEnv* env, jobject javaObject) { - AStatsManager_unregisterPullAtomCallback(android::util::GRAPHICS_STATS); + AStatsManager_clearPullAtomCallback(android::util::GRAPHICS_STATS); env->DeleteGlobalRef(gGraphicsStatsServiceObject); gGraphicsStatsServiceObject = nullptr; } diff --git a/services/core/jni/com_android_server_stats_pull_StatsPullAtomService.cpp b/services/core/jni/com_android_server_stats_pull_StatsPullAtomService.cpp index 43cd0a2d24747..b1fbe6461c44b 100644 --- a/services/core/jni/com_android_server_stats_pull_StatsPullAtomService.cpp +++ b/services/core/jni/com_android_server_stats_pull_StatsPullAtomService.cpp @@ -46,17 +46,15 @@ static AStatsManager_PullAtomCallbackReturn subsystemSleepStateCallback(int32_t static void nativeInit(JNIEnv* env, jobject javaObject) { // on device power measurement gPowerStatsPuller = server::stats::PowerStatsPuller(); - AStatsManager_registerPullAtomCallback(android::util::ON_DEVICE_POWER_MEASUREMENT, - onDevicePowerMeasurementCallback, - /* metadata= */ nullptr, - /* cookie= */ nullptr); + AStatsManager_setPullAtomCallback(android::util::ON_DEVICE_POWER_MEASUREMENT, + /* metadata= */ nullptr, onDevicePowerMeasurementCallback, + /* cookie= */ nullptr); // subsystem sleep state gSubsystemSleepStatePuller = server::stats::SubsystemSleepStatePuller(); - AStatsManager_registerPullAtomCallback(android::util::SUBSYSTEM_SLEEP_STATE, - subsystemSleepStateCallback, - /* metadata= */ nullptr, - /* cookie= */ nullptr); + AStatsManager_setPullAtomCallback(android::util::SUBSYSTEM_SLEEP_STATE, + /* metadata= */ nullptr, subsystemSleepStateCallback, + /* cookie= */ nullptr); } static const JNINativeMethod sMethods[] = {{"nativeInit", "()V", (void*)nativeInit}};