Skip to content

Commit

Permalink
[omnibox][perf] Fix 3 histograms.
Browse files Browse the repository at this point in the history
crrev.com/c/4273874 added a `Omnibox.OmniboxEditModelOpenMatchTime`
histogram, but the code was logging to `Omnibox.OpenMatchTime`, making
the data unavailable in UMA.

This CL fixes the typo.

This CL also updates the buckets of
`Omnibox.SuggestionUsed.Search.InputToNavigationStart2` &
`Omnibox.SuggestionUsed.URL.InputToNavigationStart2`. Previously, they
covered [10ms, 10minutes]. But 30% of events fell in the underflow
bucket (55% in an experiment), and 99.999% of events fell in the <14s
bucket [1].

This CL updates the histograms to cover [1ms, 1minute] and renames them
(`...InputToNavigationStart` -> `...InputToNavigationStart2`).


[1]
https://uma.googleplex.com/p/chrome/timeline_v2?sid=3850b51de4e1b9122d37fbb6a169b1a6

Bug: 1418490
Change-Id: Icbad05e77f1122ae15f4b54e2610b0ece9feb568
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4370582
Reviewed-by: Ryan Sturm <ryansturm@chromium.org>
Commit-Queue: Ryan Sturm <ryansturm@chromium.org>
Commit-Queue: manuk hovanesian <manukh@chromium.org>
Auto-Submit: manuk hovanesian <manukh@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1121977}
  • Loading branch information
manukh authored and Chromium LUCI CQ committed Mar 24, 2023
1 parent 00394c0 commit bc5575d
Show file tree
Hide file tree
Showing 4 changed files with 21 additions and 10 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,9 @@
namespace {

const char kSearchInputToNavigationStart[] =
"Omnibox.SuggestionUsed.Search.InputToNavigationStart";
"Omnibox.SuggestionUsed.Search.InputToNavigationStart2";
const char kURLInputToNavigationStart[] =
"Omnibox.SuggestionUsed.URL.InputToNavigationStart";
"Omnibox.SuggestionUsed.URL.InputToNavigationStart2";

const char kSearchFirstContentfulPaint[] =
"Omnibox.SuggestionUsed.Search.NavigationToFirstContentfulPaint";
Expand Down Expand Up @@ -84,15 +84,21 @@ void OmniboxSuggestionUsedMetricsObserver::OnFirstContentfulPaintInPage(
if (ui::PageTransitionCoreTypeIs(transition_type_,
ui::PAGE_TRANSITION_GENERATED)) {
if (timing.input_to_navigation_start) {
PAGE_LOAD_HISTOGRAM(kSearchInputToNavigationStart,
timing.input_to_navigation_start.value());
// Use `PAGE_LOAD_SHORT_HISTOGRAM()`, and not `PAGE_LOAD_HISTOGRAM()`,
// as this has many (30-55%) of events <10ms, and almost no events
// >1minute.
PAGE_LOAD_SHORT_HISTOGRAM(kSearchInputToNavigationStart,
timing.input_to_navigation_start.value());
}
PAGE_LOAD_HISTOGRAM(kSearchFirstContentfulPaint, fcp);
} else if (ui::PageTransitionCoreTypeIs(transition_type_,
ui::PAGE_TRANSITION_TYPED)) {
if (timing.input_to_navigation_start) {
PAGE_LOAD_HISTOGRAM(kURLInputToNavigationStart,
timing.input_to_navigation_start.value());
// Use `PAGE_LOAD_SHORT_HISTOGRAM()`, and not `PAGE_LOAD_HISTOGRAM()`,
// as this has many (30-55%) of events <10ms, and almost no events
// >1minute.
PAGE_LOAD_SHORT_HISTOGRAM(kURLInputToNavigationStart,
timing.input_to_navigation_start.value());
}
PAGE_LOAD_HISTOGRAM(kURLFirstContentfulPaint, fcp);
}
Expand Down
2 changes: 1 addition & 1 deletion components/omnibox/browser/omnibox_edit_model.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2234,7 +2234,7 @@ void OmniboxEditModel::OpenMatch(AutocompleteMatch match,

// TODO(manukh): Remove this histogram when `kRedoCurrentMatch` &
// `kRevertModelBeforeClosingPopup` launch or are abandoned.
SCOPED_UMA_HISTOGRAM_TIMER_MICROS("Omnibox.OpenMatchTime");
SCOPED_UMA_HISTOGRAM_TIMER_MICROS("Omnibox.OmniboxEditModelOpenMatchTime");

// Switch the window disposition to SWITCH_TO_TAB for open tab matches that
// originated while in keyword mode. This is to support the keyword mode
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,12 @@
// Up to 10 minutes, with 100 buckets.
#define PAGE_LOAD_HISTOGRAM(name, sample) \
UMA_HISTOGRAM_CUSTOM_TIMES(name, sample, base::Milliseconds(10), \
base::Minutes(10), 100)
base::Minutes(10), 100) \
\
// 1 ms to 1 minute, with 100 buckets.
#define PAGE_LOAD_SHORT_HISTOGRAM(name, sample) \
UMA_HISTOGRAM_CUSTOM_TIMES(name, sample, base::Milliseconds(1), \
base::Minutes(1), 100)

// Up to 1 hour, with 100 buckets.
#define PAGE_LOAD_LONG_HISTOGRAM(name, sample) \
Expand Down
4 changes: 2 additions & 2 deletions tools/metrics/histograms/metadata/omnibox/histograms.xml
Original file line number Diff line number Diff line change
Expand Up @@ -1917,7 +1917,7 @@ chromium-metrics-reviews@google.com.
</summary>
</histogram>

<histogram name="Omnibox.SuggestionUsed.Search.InputToNavigationStart"
<histogram name="Omnibox.SuggestionUsed.Search.InputToNavigationStart2"
units="ms" expires_after="2023-08-20">
<owner>spelchat@chromium.org</owner>
<owner>chrome-brapp-loading@google.com</owner>
Expand Down Expand Up @@ -2012,7 +2012,7 @@ chromium-metrics-reviews@google.com.
</summary>
</histogram>

<histogram name="Omnibox.SuggestionUsed.URL.InputToNavigationStart" units="ms"
<histogram name="Omnibox.SuggestionUsed.URL.InputToNavigationStart2" units="ms"
expires_after="2023-08-20">
<owner>spelchat@chromium.org</owner>
<owner>chrome-brapp-loading@google.com</owner>
Expand Down

0 comments on commit bc5575d

Please sign in to comment.