New issue
Advanced search Search tips

Issue 897590 link

Starred by 2 users

Issue metadata

Status: Verified
Owner:
Closed: Dec 10
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Feature



Sign in to add a comment

Only Rank Oldest N Tabs for TabRanker

Project Member Reported by charleszhao@chromium.org, Oct 22

Issue description

Current TabRanker 
https://cs.chromium.org/chromium/src/chrome/browser/resource_coordinator/tab_manager.cc?rcl=036350672c7cc81d99da59975900b7a07331aaea&l=276

either rerank all tabs (Enabled) or none (Disabled).

But the TabManager only discards one tab very time.

So there could be the a potential problem:

Let us say the user has opened 1000 tabs. He is in memory pressure.
Suppose: 
(1) Tabs are sorted by least recent used as: 
    tab_0, tab_1, tab_2, ..., tab_1000
(2) Tabs are sorted by TabRanker score as:
    tab_1000, tab_20, tab_22, tab_23, tab_24, ...
(3) The user eventually reload page tab_1000, tab_2, tab_4, tab_5, and then 
    closed the browser.

In the case above, TabRanker have averagely better result if TabManager discards 5 tabs every time.

But currently TabManager discards one tab very time (which is meant to be), then there is a chance the bad first one makes the TabRanker result much worse.

Currently it's hard to verify this unless we reconstruct a users' full tab list.

But it can be very helpful if we have the options to set how many tabs to rerank.



 
Description: Show this description
Summary: Only Rank Oldest N Tabs for TabRanker (was: Only Rank TopN for TabRanker)
Description: Show this description
Description: Show this description
Description: Show this description
Cc: fdoray@chromium.org
Project Member

Comment 7 by bugdroid1@chromium.org, Oct 25

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0669c3d40b0789c6f94e7967821f422e46721630

commit 0669c3d40b0789c6f94e7967821f422e46721630
Author: Charles Zhao <charleszhao@chromium.org>
Date: Thu Oct 25 06:48:09 2018

Only re-rank oldest N tabs with TabRanker.

We use least recently used index:
  lru_index = mru.total - mru.index - 1
To only Score the oldest N tabs.
N is configurable from Finch and defaulted as kMaxInt.

The reason for this change:
Since TabManager discards one tab every time, we suspect any error of
TabRanker among all tabs can make a bad first candidate which leads to
not so good tab discarding result.
By only re-rank oldest N tabs, we can shrink the chance of making such
mistakes (in the mean time, less likely to find the best candidate to
discards).

Related histogram:
    "TabManager.Discarding.ReloadCount", (expect to go down)
    "TabManager.Discarding.DiscardToReloadTime", (expect to go up)

Bug:897590

Change-Id: I76d51605cea75f5db28250594bba35ae0a59cae5
Reviewed-on: https://chromium-review.googlesource.com/c/1293060
Commit-Queue: Charles . <charleszhao@chromium.org>
Reviewed-by: François Doray <fdoray@chromium.org>
Reviewed-by: Michael Giuffrida <michaelpg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#602625}
[modify] https://crrev.com/0669c3d40b0789c6f94e7967821f422e46721630/chrome/browser/resource_coordinator/tab_activity_watcher.cc
[modify] https://crrev.com/0669c3d40b0789c6f94e7967821f422e46721630/chrome/browser/resource_coordinator/tab_activity_watcher_browsertest.cc
[modify] https://crrev.com/0669c3d40b0789c6f94e7967821f422e46721630/chrome/browser/resource_coordinator/tab_manager_features.cc
[modify] https://crrev.com/0669c3d40b0789c6f94e7967821f422e46721630/chrome/browser/resource_coordinator/tab_manager_features.h

Status: Verified (was: Started)
What are the results of this work? Did histograms or other observations moved in the expected direction?

Sign in to add a comment