TabDiscarder logging at query time. |
|
Issue descriptionCurrently TabRanker is not performing as expected, we suspect “Data distributions may be significantly different from full data to discarded only data” So we propose this new logging to address this issue. (1) Logging at query time. When a user is under memory pressure, the TabManager will ask a list of candidates to discard, which is based on Least recently used (if TabRanker is disabled) or reactivation score (if TabRanker is enabled). We will log all features when TabRanker scores a tab, and log the corresponding label when the tab is later on reactivated or closed. (2) Combining features and labels with LabelId. A random int64 number is generated, saved in WebContentsData and logged to UKM with other features every time when a tab is scored. The same number is logged with labels when the tab is reactivated or closed. By doing this, we will get a more reliable way to combine features and labels. (3) Combining tabs with QueryId. A random int64 number (different from above) is generated, saved in TabActivityWatcher and logged to UKM with other features every time when the GetSortedLifecycleUnits() is called. By doing this, all tabs that logged for that query can be later on joined as a full list of all tabs. This allows us to analyze and compare the difference between any trained model and current Least Recently Used approach.
,
Nov 22
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/5128eeb548df6435962f53401f609e02c8e95119 commit 5128eeb548df6435962f53401f609e02c8e95119 Author: Charles Zhao <charleszhao@chromium.org> Date: Thu Nov 22 08:03:55 2018 TabRanker: Refactor of TabFeatures to be ready for query time logging. In order to do query time logging, WindowFeatures and MRUFeatures have to be merged into current TabFeatures. Which also leads to modify three other places: (1) how TabFeatures is calculated in TabActivityWatcher. (2) how TabFeatures is used in CalculateReactivationScore. (3) how TabFeatures is logged to ukm. To be more specific of the change: (1) Extending TabFeatures: chrome/browser/resource_coordinator/tab_ranker/tab_features.h chrome/browser/resource_coordinator/tab_ranker/tab_features.cc chrome/browser/resource_coordinator/tab_ranker/tab_features_unittest.cc 6 extra fields is added to TabFeatures (4 from WindowFeatures, 2 from MRUFeatures). 2 helper functions are added PopulateTabFeaturesToRankerExample (for inference) PopulateTabFeaturesToUkmEntry (for logging) 2) tab_score_predictor tab_score_predictor can simply take TabFeatures as input. chrome/browser/resource_coordinator/tab_ranker/tab_score_predictor.h chrome/browser/resource_coordinator/tab_ranker/tab_score_predictor.cc chrome/browser/resource_coordinator/tab_ranker/tab_score_predictor_unittest.cc (3) For slight different logging. chrome/browser/resource_coordinator/tab_metrics_logger.h chrome/browser/resource_coordinator/tab_metrics_logger.cc chrome/browser/resource_coordinator/tab_metrics_logger_unittest.cc LogBackgroundTab is changed to LogTabMetrics which basically logs a full TabFeatures. LogBackgroundTabShown and LogBackgroundTabClosed are also unified as LogForegroundedOrClosedMetrics to avoid getting more and more parameters. (4) For Triggering of logging. chrome/browser/resource_coordinator/tab_activity_watcher.h chrome/browser/resource_coordinator/tab_activity_watcher.cc chrome/browser/resource_coordinator/tab_activity_watcher_browsertest.cc The triggering logic is not changed. Only two helper functions are added: GetTabFeatures (returns TabFeatures of current tab) LogForegroundedOrClosedMetrics (gets ForegroundedOrClosed metrics of current tab and logs to ukm.) 5) tools/metrics/ukm/ukm.xml 8 extra metrics are added to "TabManager.TabMetrics", among which: "MRUIndex", "TotalTabCount", "TimeFromBackgrounded" are moved from ForegroundedOrClosed event. "WindowIsActive", "WindowShowState", "WindowTabCount", "WindowType" are moved from WindowMetrics event. "NumReactivationBefore" is the only newly added event which is currently estimated offline based on TabMetrics; now we can have more precise value by directly logging it. Bug: 900454 Change-Id: I705c1c144be4c3df0d5b67a7fe5f2d1afa10f2c9 Reviewed-on: https://chromium-review.googlesource.com/c/1337139 Reviewed-by: Kentaro Hara <haraken@chromium.org> Reviewed-by: Robert Kaplow <rkaplow@chromium.org> Reviewed-by: Michael Giuffrida <michaelpg@chromium.org> Reviewed-by: Sébastien Marchand <sebmarchand@chromium.org> Commit-Queue: Charles . <charleszhao@chromium.org> Cr-Commit-Position: refs/heads/master@{#610347} [modify] https://crrev.com/5128eeb548df6435962f53401f609e02c8e95119/chrome/browser/resource_coordinator/tab_activity_watcher.cc [modify] https://crrev.com/5128eeb548df6435962f53401f609e02c8e95119/chrome/browser/resource_coordinator/tab_activity_watcher_browsertest.cc [modify] https://crrev.com/5128eeb548df6435962f53401f609e02c8e95119/chrome/browser/resource_coordinator/tab_metrics_logger.cc [modify] https://crrev.com/5128eeb548df6435962f53401f609e02c8e95119/chrome/browser/resource_coordinator/tab_metrics_logger.h [modify] https://crrev.com/5128eeb548df6435962f53401f609e02c8e95119/chrome/browser/resource_coordinator/tab_metrics_logger_unittest.cc [modify] https://crrev.com/5128eeb548df6435962f53401f609e02c8e95119/chrome/browser/resource_coordinator/tab_ranker/BUILD.gn [modify] https://crrev.com/5128eeb548df6435962f53401f609e02c8e95119/chrome/browser/resource_coordinator/tab_ranker/tab_features.cc [modify] https://crrev.com/5128eeb548df6435962f53401f609e02c8e95119/chrome/browser/resource_coordinator/tab_ranker/tab_features.h [add] https://crrev.com/5128eeb548df6435962f53401f609e02c8e95119/chrome/browser/resource_coordinator/tab_ranker/tab_features_test_helper.cc [add] https://crrev.com/5128eeb548df6435962f53401f609e02c8e95119/chrome/browser/resource_coordinator/tab_ranker/tab_features_test_helper.h [add] https://crrev.com/5128eeb548df6435962f53401f609e02c8e95119/chrome/browser/resource_coordinator/tab_ranker/tab_features_unittest.cc [modify] https://crrev.com/5128eeb548df6435962f53401f609e02c8e95119/chrome/browser/resource_coordinator/tab_ranker/tab_score_predictor.cc [modify] https://crrev.com/5128eeb548df6435962f53401f609e02c8e95119/chrome/browser/resource_coordinator/tab_ranker/tab_score_predictor.h [modify] https://crrev.com/5128eeb548df6435962f53401f609e02c8e95119/chrome/browser/resource_coordinator/tab_ranker/tab_score_predictor_unittest.cc [modify] https://crrev.com/5128eeb548df6435962f53401f609e02c8e95119/chrome/test/BUILD.gn [modify] https://crrev.com/5128eeb548df6435962f53401f609e02c8e95119/tools/metrics/ukm/ukm.xml
,
Nov 29
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/3e8927839919e847328389775c5df58b798c7fd3 commit 3e8927839919e847328389775c5df58b798c7fd3 Author: Charles Zhao <charleszhao@chromium.org> Date: Thu Nov 29 06:42:07 2018 TabRanker: Query time logging with label_id and query_id. When chrome browser is under memory pressure, TabManager::GetSortedLifecycleUnits is called to return list of tabs sorted by its reactivation score. We want to log this event by: (1) generates (and logs) a query_id for this query. (2) for each tab, when its reactivation score is calculated, we log its tab_features (with a random label_id) for this query. (3) when the tab is later on reactivated or closed, we log the label with the label_id generated before so that feature-label can be paired. Please see more details in the bug. Changes made in this CL: (1) Extending TabMetricsLogger chrome/browser/resource_coordinator/tab_metrics_logger.h chrome/browser/resource_coordinator/tab_metrics_logger.cc chrome/browser/resource_coordinator/tab_metrics_logger_unittest.cc Minor change of these functions to also take label_id and query_id as input. (label_id is passed in as input since it's different per tab, query_id is saved and set inside TabMetricsLogger since it's the same of all tabs per discard-query) (2) Populating the label_id and query_id chrome/browser/resource_coordinator/tab_activity_watcher.h chrome/browser/resource_coordinator/tab_activity_watcher.cc chrome/browser/resource_coordinator/tab_activity_watcher_browsertest.cc TabActivityWatcher::CalculateReactivationScore and TabActivityWatcher::WebContentsData::CalculateReactivationScore are extended to also take also_log_to_ukm as input. A tab_feature with a random label_id_ is logged whenever CalculateReactivationScore(true) is called. The same label_id_ is also logged in LogForegroundedOrClosedMetrics. We also add logic to block background time feature logging for now. (3) TabManager changes. chrome/browser/resource_coordinator/tab_lifecycle_unit.cc chrome/browser/resource_coordinator/tab_manager.h chrome/browser/resource_coordinator/tab_manager.cc These changes are necessary only because TabManager::GetSortedLifecycleUnits and TabActivityWatcher::CalculateReactivationScore are called at multiple places. And we only want to log some of them. a) TabLifecycleUnitSource::TabLifecycleUnit::GetSortKey is changed back to only sort by last_activity_time for now. b) is added as private function and called inside TabManager::DiscardTabImpl if TabRanker is enabled. c) CalculateReactivationScore(also_log_to_ukm=true) is called inside GetSortedLifecycleUnitsFromTabRanker, which is the only case we log query time features. Bug: 900454 Change-Id: Idc77b0ff6aa3488e10a449dfec6a96d4c61f8da0 Reviewed-on: https://chromium-review.googlesource.com/c/1350445 Reviewed-by: Kentaro Hara <haraken@chromium.org> Reviewed-by: Michael Giuffrida <michaelpg@chromium.org> Reviewed-by: Robert Kaplow <rkaplow@chromium.org> Reviewed-by: Sébastien Marchand <sebmarchand@chromium.org> Commit-Queue: Charles . <charleszhao@chromium.org> Cr-Commit-Position: refs/heads/master@{#612070} [modify] https://crrev.com/3e8927839919e847328389775c5df58b798c7fd3/chrome/browser/resource_coordinator/tab_activity_watcher.cc [modify] https://crrev.com/3e8927839919e847328389775c5df58b798c7fd3/chrome/browser/resource_coordinator/tab_activity_watcher.h [modify] https://crrev.com/3e8927839919e847328389775c5df58b798c7fd3/chrome/browser/resource_coordinator/tab_activity_watcher_browsertest.cc [modify] https://crrev.com/3e8927839919e847328389775c5df58b798c7fd3/chrome/browser/resource_coordinator/tab_lifecycle_unit.cc [modify] https://crrev.com/3e8927839919e847328389775c5df58b798c7fd3/chrome/browser/resource_coordinator/tab_manager.cc [modify] https://crrev.com/3e8927839919e847328389775c5df58b798c7fd3/chrome/browser/resource_coordinator/tab_manager.h [modify] https://crrev.com/3e8927839919e847328389775c5df58b798c7fd3/chrome/browser/resource_coordinator/tab_manager_features.cc [modify] https://crrev.com/3e8927839919e847328389775c5df58b798c7fd3/chrome/browser/resource_coordinator/tab_manager_features.h [modify] https://crrev.com/3e8927839919e847328389775c5df58b798c7fd3/chrome/browser/resource_coordinator/tab_metrics_logger.cc [modify] https://crrev.com/3e8927839919e847328389775c5df58b798c7fd3/chrome/browser/resource_coordinator/tab_metrics_logger.h [modify] https://crrev.com/3e8927839919e847328389775c5df58b798c7fd3/chrome/browser/resource_coordinator/tab_metrics_logger_unittest.cc [modify] https://crrev.com/3e8927839919e847328389775c5df58b798c7fd3/tools/metrics/ukm/ukm.xml |
|
►
Sign in to add a comment |
|
Comment 1 by charleszhao@chromium.org
, Oct 31These three flags are added to give us more control of logging and inference. int GetNumOldestTabsToScoreWithTabRanker() int GetNumOldestTabsToLogWithTabRanker() bool DisableBackgroundLogWithTabRanker() a) NumOldestTabsToScoreWithTabRanker let us rerank a subset of tabs with tabranker. b) DisableBackgroundLogWithTabRanker let us disable background log. c) NumOldestTabsToLogWithTabRanker let us log a subset of tabs at query time. Let us consider these tree cases: i) DisableBackgroundLogWithTabRanker == false && NumOldestTabsToLogWithTabRanker==0 This is the default setup, which is current setup. Old logging is enabled, and new logging is disabled. ii)DisableBackgroundLogWithTabRanker == true && NumOldestTabsToLogWithTabRanker==20 This is intended way to use it. This will completely disable the old tabranker logging. but enable query time logging for the oldest 20 tabs. iii) DisableBackgroundLogWithTabRanker == false && NumOldestTabsToLogWithTabRanker==20 This will enable old logging and new logging at the same time. The offline pipeline can easily distinguish them with label_id. All events with label_id!=0 are new logging, and all events with label_id==0 are old logging.