Merge "Debounce GroupCoalescer instead of using single timeout"
diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coalescer/EventBatch.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coalescer/EventBatch.java
index 2c6a165..2eec68b 100644
--- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coalescer/EventBatch.java
+++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coalescer/EventBatch.java
@@ -16,6 +16,8 @@
package com.android.systemui.statusbar.notification.collection.coalescer;
+import androidx.annotation.Nullable;
+
import java.util.ArrayList;
import java.util.List;
@@ -35,6 +37,8 @@
*/
final List<CoalescedEvent> mMembers = new ArrayList<>();
+ @Nullable Runnable mCancelShortTimeout;
+
EventBatch(long createdTimestamp, String groupKey) {
mCreatedTimestamp = createdTimestamp;
this.mGroupKey = groupKey;
diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coalescer/GroupCoalescer.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coalescer/GroupCoalescer.java
index 8076616..f589038 100644
--- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coalescer/GroupCoalescer.java
+++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coalescer/GroupCoalescer.java
@@ -16,6 +16,7 @@
package com.android.systemui.statusbar.notification.collection.coalescer;
+import static com.android.systemui.statusbar.notification.logging.NotifEvent.BATCH_MAX_TIMEOUT;
import static com.android.systemui.statusbar.notification.logging.NotifEvent.COALESCED_EVENT;
import static com.android.systemui.statusbar.notification.logging.NotifEvent.EARLY_BATCH_EMIT;
import static com.android.systemui.statusbar.notification.logging.NotifEvent.EMIT_EVENT_BATCH;
@@ -71,7 +72,8 @@
private final DelayableExecutor mMainExecutor;
private final SystemClock mClock;
private final NotifLog mLog;
- private final long mGroupLingerDuration;
+ private final long mMinGroupLingerDuration;
+ private final long mMaxGroupLingerDuration;
private BatchableNotificationHandler mHandler;
@@ -82,22 +84,28 @@
public GroupCoalescer(
@Main DelayableExecutor mainExecutor,
SystemClock clock, NotifLog log) {
- this(mainExecutor, clock, log, GROUP_LINGER_DURATION);
+ this(mainExecutor, clock, log, MIN_GROUP_LINGER_DURATION, MAX_GROUP_LINGER_DURATION);
}
/**
- * @param groupLingerDuration How long, in ms, that notifications that are members of a group
- * are delayed within the GroupCoalescer before being posted
+ * @param minGroupLingerDuration How long, in ms, to wait for another notification from the same
+ * group to arrive before emitting all pending events for that
+ * group. Each subsequent arrival of a group member resets the
+ * timer for that group.
+ * @param maxGroupLingerDuration The maximum time, in ms, that a group can linger in the
+ * coalescer before it's force-emitted.
*/
GroupCoalescer(
@Main DelayableExecutor mainExecutor,
SystemClock clock,
NotifLog log,
- long groupLingerDuration) {
+ long minGroupLingerDuration,
+ long maxGroupLingerDuration) {
mMainExecutor = mainExecutor;
mClock = clock;
mLog = log;
- mGroupLingerDuration = groupLingerDuration;
+ mMinGroupLingerDuration = minGroupLingerDuration;
+ mMaxGroupLingerDuration = maxGroupLingerDuration;
}
/**
@@ -115,7 +123,7 @@
private final NotificationHandler mListener = new NotificationHandler() {
@Override
public void onNotificationPosted(StatusBarNotification sbn, RankingMap rankingMap) {
- maybeEmitBatch(sbn.getKey());
+ maybeEmitBatch(sbn);
applyRanking(rankingMap);
final boolean shouldCoalesce = handleNotificationPosted(sbn, rankingMap);
@@ -130,7 +138,7 @@
@Override
public void onNotificationRemoved(StatusBarNotification sbn, RankingMap rankingMap) {
- maybeEmitBatch(sbn.getKey());
+ maybeEmitBatch(sbn);
applyRanking(rankingMap);
mHandler.onNotificationRemoved(sbn, rankingMap);
}
@@ -140,7 +148,7 @@
StatusBarNotification sbn,
RankingMap rankingMap,
int reason) {
- maybeEmitBatch(sbn.getKey());
+ maybeEmitBatch(sbn);
applyRanking(rankingMap);
mHandler.onNotificationRemoved(sbn, rankingMap, reason);
}
@@ -152,13 +160,20 @@
}
};
- private void maybeEmitBatch(String memberKey) {
- CoalescedEvent event = mCoalescedEvents.get(memberKey);
+ private void maybeEmitBatch(StatusBarNotification sbn) {
+ final CoalescedEvent event = mCoalescedEvents.get(sbn.getKey());
+ final EventBatch batch = mBatches.get(sbn.getGroupKey());
if (event != null) {
mLog.log(EARLY_BATCH_EMIT,
String.format("Modification of %s triggered early emit of batched group %s",
- memberKey, requireNonNull(event.getBatch()).mGroupKey));
+ sbn.getKey(), requireNonNull(event.getBatch()).mGroupKey));
emitBatch(requireNonNull(event.getBatch()));
+ } else if (batch != null
+ && mClock.uptimeMillis() - batch.mCreatedTimestamp >= mMaxGroupLingerDuration) {
+ mLog.log(BATCH_MAX_TIMEOUT,
+ String.format("Modification of %s triggered timeout emit of batched group %s",
+ sbn.getKey(), batch.mGroupKey));
+ emitBatch(batch);
}
}
@@ -175,7 +190,8 @@
}
if (sbn.isGroup()) {
- EventBatch batch = startBatchingGroup(sbn.getGroupKey());
+ final EventBatch batch = getOrBuildBatch(sbn.getGroupKey());
+
CoalescedEvent event =
new CoalescedEvent(
sbn.getKey(),
@@ -183,10 +199,10 @@
sbn,
requireRanking(rankingMap, sbn.getKey()),
batch);
+ mCoalescedEvents.put(event.getKey(), event);
batch.mMembers.add(event);
-
- mCoalescedEvents.put(event.getKey(), event);
+ resetShortTimeout(batch);
return true;
} else {
@@ -194,27 +210,39 @@
}
}
- private EventBatch startBatchingGroup(final String groupKey) {
+ private EventBatch getOrBuildBatch(final String groupKey) {
EventBatch batch = mBatches.get(groupKey);
if (batch == null) {
- final EventBatch newBatch = new EventBatch(mClock.uptimeMillis(), groupKey);
- mBatches.put(groupKey, newBatch);
- mMainExecutor.executeDelayed(() -> emitBatch(newBatch), mGroupLingerDuration);
-
- batch = newBatch;
+ batch = new EventBatch(mClock.uptimeMillis(), groupKey);
+ mBatches.put(groupKey, batch);
}
return batch;
}
+ private void resetShortTimeout(EventBatch batch) {
+ if (batch.mCancelShortTimeout != null) {
+ batch.mCancelShortTimeout.run();
+ }
+ batch.mCancelShortTimeout =
+ mMainExecutor.executeDelayed(
+ () -> {
+ batch.mCancelShortTimeout = null;
+ emitBatch(batch);
+ },
+ mMinGroupLingerDuration);
+ }
+
private void emitBatch(EventBatch batch) {
if (batch != mBatches.get(batch.mGroupKey)) {
- // If we emit a batch early, we don't want to emit it a second time when its timeout
- // expires.
- return;
+ throw new IllegalStateException("Cannot emit out-of-date batch " + batch.mGroupKey);
}
if (batch.mMembers.isEmpty()) {
throw new IllegalStateException("Batch " + batch.mGroupKey + " cannot be empty");
}
+ if (batch.mCancelShortTimeout != null) {
+ batch.mCancelShortTimeout.run();
+ batch.mCancelShortTimeout = null;
+ }
mBatches.remove(batch.mGroupKey);
@@ -299,5 +327,6 @@
void onNotificationBatchPosted(List<CoalescedEvent> events);
}
- private static final int GROUP_LINGER_DURATION = 500;
+ private static final int MIN_GROUP_LINGER_DURATION = 50;
+ private static final int MAX_GROUP_LINGER_DURATION = 500;
}
diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/logging/NotifEvent.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/logging/NotifEvent.java
index 2374cde..9adceb7 100644
--- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/logging/NotifEvent.java
+++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/logging/NotifEvent.java
@@ -156,7 +156,8 @@
// GroupCoalescer labels:
"CoalescedEvent",
"EarlyBatchEmit",
- "EmitEventBatch"
+ "EmitEventBatch",
+ "BatchMaxTimeout"
};
private static final int TOTAL_EVENT_LABELS = EVENT_LABELS.length;
@@ -206,5 +207,6 @@
public static final int COALESCED_EVENT = COALESCER_EVENT_START_INDEX;
public static final int EARLY_BATCH_EMIT = COALESCER_EVENT_START_INDEX + 1;
public static final int EMIT_EVENT_BATCH = COALESCER_EVENT_START_INDEX + 2;
+ public static final int BATCH_MAX_TIMEOUT = COALESCER_EVENT_START_INDEX + 3;
private static final int TOTAL_COALESCER_EVENT_TYPES = 3;
}
diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/coalescer/GroupCoalescerTest.java b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/coalescer/GroupCoalescerTest.java
index 5e0baf2..86c1eb97 100644
--- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/coalescer/GroupCoalescerTest.java
+++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/coalescer/GroupCoalescerTest.java
@@ -80,7 +80,8 @@
mExecutor,
mClock,
mLog,
- LINGER_DURATION);
+ MIN_LINGER_DURATION,
+ MAX_LINGER_DURATION);
mCoalescer.setNotificationHandler(mListener);
mCoalescer.attach(mListenerService);
@@ -96,7 +97,7 @@
new NotificationEntryBuilder()
.setId(0)
.setPkg(TEST_PACKAGE_A));
- mClock.advanceTime(LINGER_DURATION);
+ mClock.advanceTime(MIN_LINGER_DURATION);
// THEN the event is passed through to the handler
verify(mListener).onNotificationPosted(notif1.sbn, notif1.rankingMap);
@@ -144,12 +145,16 @@
.setId(1)
.setGroup(mContext, GROUP_1));
+ mClock.advanceTime(2);
+
NotifEvent notif2 = mNoMan.postNotif(new NotificationEntryBuilder()
.setPkg(TEST_PACKAGE_A)
.setId(2)
.setGroup(mContext, GROUP_1)
.setGroupSummary(mContext, true));
+ mClock.advanceTime(3);
+
NotifEvent notif3 = mNoMan.postNotif(new NotificationEntryBuilder()
.setPkg(TEST_PACKAGE_A)
.setId(3)
@@ -161,7 +166,7 @@
verify(mListener, never()).onNotificationBatchPosted(anyList());
// WHEN enough time passes
- mClock.advanceTime(LINGER_DURATION);
+ mClock.advanceTime(MIN_LINGER_DURATION);
// THEN the coalesced notifs are applied. The summary is sorted to the front.
verify(mListener).onNotificationBatchPosted(Arrays.asList(
@@ -212,7 +217,7 @@
// WHEN the time runs out on the remainder of the group
clearInvocations(mListener);
- mClock.advanceTime(LINGER_DURATION);
+ mClock.advanceTime(MIN_LINGER_DURATION);
// THEN no lingering batch is applied
verify(mListener, never()).onNotificationBatchPosted(anyList());
@@ -225,11 +230,13 @@
.setPkg(TEST_PACKAGE_A)
.setId(1)
.setGroup(mContext, GROUP_1));
+ mClock.advanceTime(2);
NotifEvent notif2a = mNoMan.postNotif(new NotificationEntryBuilder()
.setPkg(TEST_PACKAGE_A)
.setId(2)
.setContentTitle(mContext, "Version 1")
.setGroup(mContext, GROUP_1));
+ mClock.advanceTime(4);
// WHEN one of them gets updated
NotifEvent notif2b = mNoMan.postNotif(new NotificationEntryBuilder()
@@ -248,7 +255,7 @@
any(RankingMap.class));
// THEN second, the update is emitted
- mClock.advanceTime(LINGER_DURATION);
+ mClock.advanceTime(MIN_LINGER_DURATION);
verify(mListener).onNotificationBatchPosted(Collections.singletonList(
new CoalescedEvent(notif2b.key, 0, notif2b.sbn, notif2b.ranking, null)
));
@@ -308,14 +315,61 @@
.setId(17));
// THEN they have the new rankings when they are eventually emitted
- mClock.advanceTime(LINGER_DURATION);
+ mClock.advanceTime(MIN_LINGER_DURATION);
verify(mListener).onNotificationBatchPosted(Arrays.asList(
new CoalescedEvent(notif1.key, 0, notif1.sbn, ranking1b, null),
new CoalescedEvent(notif2.key, 1, notif2.sbn, ranking2b, null)
));
}
- private static final long LINGER_DURATION = 4700;
+ @Test
+ public void testMaxLingerDuration() {
+ // GIVEN five coalesced notifications that have collectively taken 20ms to arrive, 2ms
+ // longer than the max linger duration
+ NotifEvent notif1 = mNoMan.postNotif(new NotificationEntryBuilder()
+ .setPkg(TEST_PACKAGE_A)
+ .setId(1)
+ .setGroup(mContext, GROUP_1));
+ mClock.advanceTime(4);
+ NotifEvent notif2 = mNoMan.postNotif(new NotificationEntryBuilder()
+ .setPkg(TEST_PACKAGE_A)
+ .setId(2)
+ .setGroup(mContext, GROUP_1));
+ mClock.advanceTime(4);
+ NotifEvent notif3 = mNoMan.postNotif(new NotificationEntryBuilder()
+ .setPkg(TEST_PACKAGE_A)
+ .setId(3)
+ .setGroup(mContext, GROUP_1));
+ mClock.advanceTime(4);
+ NotifEvent notif4 = mNoMan.postNotif(new NotificationEntryBuilder()
+ .setPkg(TEST_PACKAGE_A)
+ .setId(4)
+ .setGroup(mContext, GROUP_1));
+ mClock.advanceTime(4);
+ NotifEvent notif5 = mNoMan.postNotif(new NotificationEntryBuilder()
+ .setPkg(TEST_PACKAGE_A)
+ .setId(5)
+ .setGroup(mContext, GROUP_1));
+ mClock.advanceTime(4);
+
+ // WHEN a sixth notification arrives
+ NotifEvent notif6 = mNoMan.postNotif(new NotificationEntryBuilder()
+ .setPkg(TEST_PACKAGE_A)
+ .setId(6)
+ .setGroup(mContext, GROUP_1));
+
+ // THEN the first five notifications are emitted in a batch
+ verify(mListener).onNotificationBatchPosted(Arrays.asList(
+ new CoalescedEvent(notif1.key, 0, notif1.sbn, notif1.ranking, null),
+ new CoalescedEvent(notif2.key, 1, notif2.sbn, notif2.ranking, null),
+ new CoalescedEvent(notif3.key, 2, notif3.sbn, notif3.ranking, null),
+ new CoalescedEvent(notif4.key, 3, notif4.sbn, notif4.ranking, null),
+ new CoalescedEvent(notif5.key, 4, notif5.sbn, notif5.ranking, null)
+ ));
+ }
+
+ private static final long MIN_LINGER_DURATION = 5;
+ private static final long MAX_LINGER_DURATION = 18;
private static final String TEST_PACKAGE_A = "com.test.package_a";
private static final String TEST_PACKAGE_B = "com.test.package_b";