Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 8a4c6566 authored by Matías Hernández's avatar Matías Hernández
Browse files

Remove nm_binder_perf-related counters 

This has served its purpose.

Fixes: 389918945
Test: atest NotificationManagerTest (100 iterations)
Flag: EXEMPT Deleting flag
Change-Id: I654e8e5b9cc4ba86e6219eaf41694bd80b5ce85b
parent 108521ec

core/java/android/app/INotificationManager.aidl

+0 −3
Original line number Diff line number Diff line
@@ -273,7 +273,4 @@ interface INotificationManager 
    String[] getAdjustmentDeniedPackages(int userId, String key);

    boolean isAdjustmentSupportedForPackage(int userId, String key, String pkg);

    void setAdjustmentSupportedForPackage(int userId, String key, String pkg, boolean enabled);



    // TODO: b/389918945 - Remove once nm_binder_perf flags are going to Nextfood.

    void incrementCounter(String metricId);

}

core/java/android/app/NotificationManager.java

+1 −13
Original line number Diff line number Diff line
@@ -685,10 +685,8 @@ public class NotificationManager { 


    private final InstantSource mClock;

    private final RateLimiter mUpdateRateLimiter = new RateLimiter("notify (update)",

            "notifications.value_client_throttled_notify_update",

            MAX_NOTIFICATION_UPDATE_RATE);

    private final RateLimiter mUnnecessaryCancelRateLimiter = new RateLimiter("cancel (dupe)",

            "notifications.value_client_throttled_cancel_duplicate",

            MAX_NOTIFICATION_UNNECESSARY_CANCEL_RATE);

    // KnownStatus is KNOWN_STATUS_ENQUEUED/_CANCELLED

    private record KnownNotification(int knownStatus, OptionalInt progressState) {}
@@ -893,16 +891,14 @@ public class NotificationManager { 
        private final RateEstimator mInputRateEstimator;

        private final RateEstimator mOutputRateEstimator;

        private final String mName;

        private final String mCounterName;

        private final float mLimitRate;



        private Instant mLogSilencedUntil;



        private RateLimiter(String name, String counterName, float limitRate) {

        private RateLimiter(String name, float limitRate) {

            mInputRateEstimator = new RateEstimator();

            mOutputRateEstimator = new RateEstimator();

            mName = name;

            mCounterName = counterName;

            mLimitRate = limitRate;

        }
@@ -922,14 +918,6 @@ public class NotificationManager { 
                return;

            }



            if (Flags.nmBinderPerfLogNmThrottling()) {

                try {

                    service().incrementCounter(mCounterName);

                } catch (RemoteException e) {

                    Slog.w(TAG, "Ignoring error while trying to log " + mCounterName, e);

                }

            }



            long nowMillis = now.toEpochMilli();

            Slog.w(TAG, TextUtils.formatSimple(

                    "Shedding %s of %s, rate limit (%s) exceeded: input %s, output would be %s",

core/java/android/app/notification.aconfig

+0 −10
Original line number Diff line number Diff line
@@ -189,16 +189,6 @@ flag { 
  bug: "362981561"

}



flag {

  name: "nm_binder_perf_log_nm_throttling"

  namespace: "systemui"

  description: "Log throttled operations (notify, cancel) to statsd. This flag will NOT be pushed past Trunkfood."

  bug: "389918945"

  metadata {

    purpose: PURPOSE_BUGFIX

  }

}



flag {

  name: "nm_binder_perf_get_apps_with_channels"

  namespace: "systemui"

core/tests/coretests/src/android/app/NotificationManagerTest.java

+49 −10
Original line number Diff line number Diff line
@@ -17,6 +17,7 @@ 
package android.app;



import static com.google.common.truth.Truth.assertThat;

import static com.google.common.truth.Truth.assertWithMessage;



import static org.mockito.ArgumentMatchers.any;

import static org.mockito.ArgumentMatchers.anyInt;
@@ -34,6 +35,7 @@ import android.content.Context; 
import android.content.ContextWrapper;

import android.content.pm.PackageManager;

import android.content.pm.ParceledListSlice;

import android.os.ParcelFileDescriptor;

import android.os.UserHandle;

import android.platform.test.annotations.EnableFlags;

import android.platform.test.annotations.Presubmit;
@@ -42,6 +44,7 @@ import android.platform.test.flag.junit.SetFlagsRule; 
import androidx.test.core.app.ApplicationProvider;

import androidx.test.ext.junit.runners.AndroidJUnit4;

import androidx.test.filters.SmallTest;

import androidx.test.platform.app.InstrumentationRegistry;



import org.junit.After;

import org.junit.Before;
@@ -49,10 +52,15 @@ import org.junit.Rule; 
import org.junit.Test;

import org.junit.runner.RunWith;



import java.io.BufferedReader;

import java.io.IOException;

import java.io.InputStreamReader;

import java.time.Instant;

import java.time.InstantSource;

import java.util.ArrayList;

import java.util.List;

import java.util.regex.Matcher;

import java.util.regex.Pattern;



@RunWith(AndroidJUnit4.class)

@SmallTest
@@ -63,12 +71,14 @@ public class NotificationManagerTest { 


    private NotificationManagerWithMockService mNotificationManager;

    private final FakeClock mClock = new FakeClock();

    private UiAutomation mUiAutomation;



    private PackageTestableContext mContext;



    @Before

    public void setUp() {

        mContext = new PackageTestableContext(ApplicationProvider.getApplicationContext());

        mUiAutomation = InstrumentationRegistry.getInstrumentation().getUiAutomation();

        mNotificationManager = new NotificationManagerWithMockService(mContext, mClock);



        // Caches must be in test mode in order to be used in tests.
@@ -337,26 +347,24 @@ public class NotificationManagerTest { 
    }



    @Test

    @EnableFlags({Flags.FLAG_NM_BINDER_PERF_THROTTLE_NOTIFY,

            Flags.FLAG_NM_BINDER_PERF_LOG_NM_THROTTLING})

    @EnableFlags({Flags.FLAG_NM_BINDER_PERF_THROTTLE_NOTIFY})

    public void notify_rapidUpdate_logsOncePerSecond() throws Exception {

        Notification n = exampleNotification();

        clearSlog();



        for (int i = 0; i < 650; i++) {

            mNotificationManager.notify(1, n);

            mClock.advanceByMillis(10);

        }



        // Runs for a total of 6.5 seconds, so should log once (when RateEstimator catches up) + 6

        // more times (after 1 second each).

        verify(mNotificationManager.mBackendService, times(7)).incrementCounter(

                eq("notifications.value_client_throttled_notify_update"));

        assertSlogContains("Shedding notify \\(update\\) of .*, rate limit \\(.*\\) exceeded", 7);

    }



    @Test

    @EnableFlags({Flags.FLAG_NM_BINDER_PERF_THROTTLE_NOTIFY,

            Flags.FLAG_NM_BINDER_PERF_LOG_NM_THROTTLING})

    @EnableFlags({Flags.FLAG_NM_BINDER_PERF_THROTTLE_NOTIFY})

    public void cancel_unnecessaryAndRapid_logsOncePerSecond() throws Exception {

        clearSlog();



        for (int i = 0; i < 650; i++) {

            mNotificationManager.cancel(1);

            mClock.advanceByMillis(10);
@@ -364,8 +372,7 @@ public class NotificationManagerTest { 


        // Runs for a total of 6.5 seconds, so should log once (when RateEstimator catches up) + 6

        // more times (after 1 second each).

        verify(mNotificationManager.mBackendService, times(7)).incrementCounter(

                eq("notifications.value_client_throttled_cancel_duplicate"));

        assertSlogContains("Shedding cancel \\(dupe\\) of .*, rate limit \\(.*\\) exceeded", 7);

    }



    @Test
@@ -789,4 +796,36 @@ public class NotificationManagerTest { 
            mNowMillis += millis;

        }

    }



    private void clearSlog() throws Exception {

        try (ParcelFileDescriptor unused = mUiAutomation.executeShellCommand("logcat -c")) {

            Thread.sleep(500); // Give logcat time to settle.

        }

    }



    private void assertSlogContains(String entry, int times) throws Exception {

        Thread.sleep(500); // Give logcat time to settle.

        String logcat = readSlog();

        Matcher matcher = Pattern.compile(entry).matcher(logcat);

        assertWithMessage("Checking output " + logcat + " for occurrences of " + entry)

                .that(matcher.results().toList()).hasSize(times);

    }



    private String readSlog() throws IOException {

        // Dumps the system (Slog) log (-d), filters for our specific tag at Warning level (W),

        // and silences all other tags (*:S).

        String command = "logcat -d -b system NotificationManager:W *:S";

        try (ParcelFileDescriptor pfd = mUiAutomation.executeShellCommand(command)) {

            try (BufferedReader reader = new BufferedReader(

                    new InputStreamReader(

                            new ParcelFileDescriptor.AutoCloseInputStream(pfd)))) {

                StringBuilder logBuilder = new StringBuilder();

                String line;

                while ((line = reader.readLine()) != null) {

                    logBuilder.append(line).append("\n");

                }

                return logBuilder.toString();

            }

        }

    }

}

services/core/java/com/android/server/notification/NotificationManagerService.java

+0 −8
Original line number Diff line number Diff line
@@ -391,7 +391,6 @@ import com.android.internal.util.Preconditions; 
import com.android.internal.util.XmlUtils;

import com.android.internal.util.function.TriPredicate;

import com.android.internal.widget.LockPatternUtils;

import com.android.modules.expresslog.Counter;

import com.android.modules.utils.TypedXmlPullParser;

import com.android.modules.utils.TypedXmlSerializer;

import com.android.server.DeviceIdleInternal;
@@ -7640,13 +7639,6 @@ public class NotificationManagerService extends SystemService { 
            Slog.e(TAG, "exiting pullStats: bad request");

            return 0;

        }













        @Override













        public void incrementCounter(String metricId) {













            if (android.app.Flags.nmBinderPerfLogNmThrottling() && metricId != null) {













                Counter.logIncrementWithUid(metricId, Binder.getCallingUid());













            }













        }















    };





























    private void handleNotificationPermissionChange(String pkg, @UserIdInt int userId) {