From 0b710af834f5e8c593ed00bc3c46e36f0d0076fb Mon Sep 17 00:00:00 2001 From: Joshua Trask Date: Wed, 13 Oct 2021 15:12:17 -0400 Subject: [PATCH] Batch shortcut results to a single Handler message. The one-at-a-time processing model was vestigial from supporting ChooserTargetService. If we need to process n shortcuts, it's inefficient to context-switch and handle n+1 messages (including the completion message) if we can instead pack all n results into a single message. The new approach is also much simpler, since we now know we have all the data on hand before proceeding - previously we forced an "unbatched" processing model on the downstream clients, and then had to design around the problems that *that* created (in particular, I haven't confirmed yet, but I *suspect* that we could get rid of the 250ms LIST_VIEW_UPDATE_MESSAGE delay after this change, since now we can just send that message a single time at the end of the batch without worrying about throttling). Further cleanup like that will come in a later CL -- for now this is the minimum change required to adopt a batch-processing model. I claim that this change is safe because there was no benefit to the one-at-a-time streaming model -- it didn't cause results to appear to the user any faster. We already had the full set of results on hand when we started emitting these messages from ChooserActivity::sendShareShortcutInfoList; the per-result processing we do in that loop isn't very expensive (and it runs in the same thread as the ChooserHandler anyways); and even if we were somehow picking up a marginal benefit, it would've been masked by the 250ms timeout. (There *maybe* could be some concern if ChooserListAdapter's addServiceResults is too expensive for us to block and call for each result in the batch, but 1. that seems unlikely; 2. if there *is* anything expensive about addServiceResults we can probably optimize it by switching to a batch-processing model in the Adapter as well; and 3. if the Adapter needs to subdivide these tasks to remain performant, it should take that responsibility on for itself.) This optimization was made possible by the removal of ChooserTargetService support (since in the old model, we actually *didn't* have all the results on hand at the start). Test: manual & atest Change-Id: Iaaab546cc274bf1879941901260b7ade62cbcaa5 --- .../android/internal/app/ChooserActivity.java | 75 +++++++++++-------- 1 file changed, 42 insertions(+), 33 deletions(-) diff --git a/core/java/com/android/internal/app/ChooserActivity.java b/core/java/com/android/internal/app/ChooserActivity.java index 6ff74e41a56f2..9b1bef067ceca 100644 --- a/core/java/com/android/internal/app/ChooserActivity.java +++ b/core/java/com/android/internal/app/ChooserActivity.java @@ -441,14 +441,12 @@ public class ChooserActivity extends ResolverActivity implements private final ChooserHandler mChooserHandler = new ChooserHandler(); private class ChooserHandler extends Handler { - private static final int SHORTCUT_MANAGER_SHARE_TARGET_RESULT = 4; - private static final int SHORTCUT_MANAGER_SHARE_TARGET_RESULT_COMPLETED = 5; private static final int LIST_VIEW_UPDATE_MESSAGE = 6; + private static final int SHORTCUT_MANAGER_ALL_SHARE_TARGET_RESULTS = 7; private void removeAllMessages() { removeMessages(LIST_VIEW_UPDATE_MESSAGE); - removeMessages(SHORTCUT_MANAGER_SHARE_TARGET_RESULT); - removeMessages(SHORTCUT_MANAGER_SHARE_TARGET_RESULT_COMPLETED); + removeMessages(SHORTCUT_MANAGER_ALL_SHARE_TARGET_RESULTS); } @Override @@ -468,22 +466,23 @@ public class ChooserActivity extends ResolverActivity implements .refreshListView(); break; - case SHORTCUT_MANAGER_SHARE_TARGET_RESULT: - if (DEBUG) Log.d(TAG, "SHORTCUT_MANAGER_SHARE_TARGET_RESULT"); - final ServiceResultInfo resultInfo = (ServiceResultInfo) msg.obj; - if (resultInfo.resultTargets != null) { - ChooserListAdapter adapterForUserHandle = - mChooserMultiProfilePagerAdapter.getListAdapterForUserHandle( - resultInfo.userHandle); - if (adapterForUserHandle != null) { - adapterForUserHandle.addServiceResults( - resultInfo.originalTarget, resultInfo.resultTargets, msg.arg1, - mDirectShareShortcutInfoCache); + case SHORTCUT_MANAGER_ALL_SHARE_TARGET_RESULTS: + if (DEBUG) Log.d(TAG, "SHORTCUT_MANAGER_ALL_SHARE_TARGET_RESULTS"); + final ServiceResultInfo[] resultInfos = (ServiceResultInfo[]) msg.obj; + for (ServiceResultInfo resultInfo : resultInfos) { + if (resultInfo.resultTargets != null) { + ChooserListAdapter adapterForUserHandle = + mChooserMultiProfilePagerAdapter.getListAdapterForUserHandle( + resultInfo.userHandle); + if (adapterForUserHandle != null) { + adapterForUserHandle.addServiceResults( + resultInfo.originalTarget, + resultInfo.resultTargets, msg.arg1, + mDirectShareShortcutInfoCache); + } } } - break; - case SHORTCUT_MANAGER_SHARE_TARGET_RESULT_COMPLETED: logDirectShareTargetReceived( MetricsEvent.ACTION_DIRECT_SHARE_TARGETS_LOADED_SHORTCUT_MANAGER); sendVoiceChoicesIfNeeded(); @@ -1954,34 +1953,44 @@ public class ChooserActivity extends ResolverActivity implements // Match ShareShortcutInfos with DisplayResolveInfos to be able to use the old code path // for direct share targets. After ShareSheet is refactored we should use the // ShareShortcutInfos directly. + List resultRecords = new ArrayList<>(); for (int i = 0; i < chooserListAdapter.getDisplayResolveInfoCount(); i++) { - List matchingShortcuts = new ArrayList<>(); - for (int j = 0; j < resultList.size(); j++) { - if (chooserListAdapter.getDisplayResolveInfo(i).getResolvedComponentName().equals( - resultList.get(j).getTargetComponent())) { - matchingShortcuts.add(resultList.get(j)); - } - } + DisplayResolveInfo displayResolveInfo = chooserListAdapter.getDisplayResolveInfo(i); + List matchingShortcuts = + filterShortcutsByTargetComponentName( + resultList, displayResolveInfo.getResolvedComponentName()); if (matchingShortcuts.isEmpty()) { continue; } List chooserTargets = convertToChooserTarget( matchingShortcuts, resultList, appTargets, shortcutType); - final Message msg = Message.obtain(); - msg.what = ChooserHandler.SHORTCUT_MANAGER_SHARE_TARGET_RESULT; - msg.obj = new ServiceResultInfo(chooserListAdapter.getDisplayResolveInfo(i), - chooserTargets, userHandle); - msg.arg1 = shortcutType; - mChooserHandler.sendMessage(msg); + ServiceResultInfo resultRecord = new ServiceResultInfo( + displayResolveInfo, chooserTargets, userHandle); + resultRecords.add(resultRecord); } - sendShortcutManagerShareTargetResultCompleted(); + sendShortcutManagerShareTargetResults( + shortcutType, resultRecords.toArray(new ServiceResultInfo[0])); } - private void sendShortcutManagerShareTargetResultCompleted() { + private List filterShortcutsByTargetComponentName( + List allShortcuts, ComponentName requiredTarget) { + List matchingShortcuts = new ArrayList<>(); + for (ShortcutManager.ShareShortcutInfo shortcut : allShortcuts) { + if (requiredTarget.equals(shortcut.getTargetComponent())) { + matchingShortcuts.add(shortcut); + } + } + return matchingShortcuts; + } + + private void sendShortcutManagerShareTargetResults( + int shortcutType, ServiceResultInfo[] results) { final Message msg = Message.obtain(); - msg.what = ChooserHandler.SHORTCUT_MANAGER_SHARE_TARGET_RESULT_COMPLETED; + msg.what = ChooserHandler.SHORTCUT_MANAGER_ALL_SHARE_TARGET_RESULTS; + msg.obj = results; + msg.arg1 = shortcutType; mChooserHandler.sendMessage(msg); }