Skip to content

Commit

Permalink
Add histogram for whether a model was available at registration
Browse files Browse the repository at this point in the history
Bug: 1279614
Change-Id: I71982787905cbf263e7322697be65f6f6e3692a2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3490655
Reviewed-by: Tarun Bansal <tbansal@chromium.org>
Commit-Queue: Sophie Chang <sophiechang@chromium.org>
Cr-Commit-Position: refs/heads/main@{#975857}
  • Loading branch information
Sophie Chang authored and Chromium LUCI CQ committed Feb 28, 2022
1 parent a05a192 commit 82c6352
Show file tree
Hide file tree
Showing 3 changed files with 86 additions and 17 deletions.
20 changes: 18 additions & 2 deletions components/optimization_guide/core/prediction_manager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,16 @@ bool IsModelMetadataTypeOnServerAllowlist(
"WebPermissionPredictionsModelMetadata";
}

void RecordModelAvailableAtRegistration(
optimization_guide::proto::OptimizationTarget optimization_target,
bool model_available_at_registration) {
base::UmaHistogramBoolean(
"OptimizationGuide.PredictionManager.ModelAvailableAtRegistration." +
optimization_guide::GetStringNameForOptimizationTarget(
optimization_target),
model_available_at_registration);
}

} // namespace

namespace optimization_guide {
Expand Down Expand Up @@ -664,6 +674,8 @@ void PredictionManager::LoadPredictionModels(
BuildPredictionModelFromCommandLineForOptimizationTarget(
optimization_target);
OnLoadPredictionModel(std::move(prediction_model));
RecordModelAvailableAtRegistration(optimization_target,
prediction_model != nullptr);
}
return;
}
Expand All @@ -675,8 +687,12 @@ void PredictionManager::LoadPredictionModels(
for (const auto& optimization_target : optimization_targets) {
// The prediction model for this optimization target has already been
// loaded.
if (!model_and_features_store_->FindPredictionModelEntryKey(
optimization_target, &model_entry_key)) {
bool model_stored_locally =
model_and_features_store_->FindPredictionModelEntryKey(
optimization_target, &model_entry_key);
RecordModelAvailableAtRegistration(optimization_target,
model_stored_locally);
if (!model_stored_locally) {
continue;
}
model_and_features_store_->LoadPredictionModel(
Expand Down
69 changes: 54 additions & 15 deletions components/optimization_guide/core/prediction_manager_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -562,6 +562,11 @@ TEST_F(PredictionManagerTest, AddObserverForOptimizationTargetModel) {
SetStoreInitialized(/* load_models= */ false,
/* have_models_in_store= */ false);

histogram_tester.ExpectUniqueSample(
"OptimizationGuide.PredictionManager.ModelAvailableAtRegistration."
"PainfulPageLoad",
false, 1);

EXPECT_TRUE(prediction_model_fetcher()->models_fetched());
// Make sure the test histogram is recorded. We don't check for value here
// since that is too much toil for someone whenever they add a new version.
Expand Down Expand Up @@ -589,21 +594,31 @@ TEST_F(PredictionManagerTest, AddObserverForOptimizationTargetModel) {
model_info.add_additional_files()->set_file_path("");

// Ensure observer is hooked up.
proto::PredictionModel model1;
*model1.mutable_model_info() = model_info;
model1.mutable_model()->set_download_url(
FilePathToString(temp_dir().AppendASCII("whatever")));
prediction_manager()->OnModelReady(model1);
RunUntilIdle();
{
base::HistogramTester model_ready_histogram_tester;

absl::optional<ModelInfo> received_model =
observer.last_received_model_for_target(
proto::OPTIMIZATION_TARGET_PAINFUL_PAGE_LOAD);
EXPECT_EQ(received_model->GetModelMetadata()->type_url(), "sometypeurl");
EXPECT_EQ(received_model->GetModelFilePath().BaseName().value(),
FILE_PATH_LITERAL("whatever"));
EXPECT_EQ(received_model->GetAdditionalFiles(),
base::flat_set<base::FilePath>{additional_file_path});
proto::PredictionModel model1;
*model1.mutable_model_info() = model_info;
model1.mutable_model()->set_download_url(
FilePathToString(temp_dir().AppendASCII("whatever")));
prediction_manager()->OnModelReady(model1);
RunUntilIdle();

absl::optional<ModelInfo> received_model =
observer.last_received_model_for_target(
proto::OPTIMIZATION_TARGET_PAINFUL_PAGE_LOAD);
EXPECT_EQ(received_model->GetModelMetadata()->type_url(), "sometypeurl");
EXPECT_EQ(received_model->GetModelFilePath().BaseName().value(),
FILE_PATH_LITERAL("whatever"));
EXPECT_EQ(received_model->GetAdditionalFiles(),
base::flat_set<base::FilePath>{additional_file_path});

// Make sure we do not record the model available histogram again.
model_ready_histogram_tester.ExpectTotalCount(
"OptimizationGuide.PredictionManager.ModelAvailableAtRegistration."
"PainfulPageLoad",
0);
}

// Reset fetcher and make sure version is sent in the new request and not
// counted as re-loaded or updated.
Expand Down Expand Up @@ -780,9 +795,15 @@ TEST_F(PredictionManagerTest,
base::HistogramTester histogram_tester;

CreatePredictionManager();
SetStoreInitialized(/*load_models=*/false, /*have_models_in_store=*/false);
FakeOptimizationTargetModelObserver observer;
prediction_manager()->AddObserverForOptimizationTargetModel(
proto::OPTIMIZATION_TARGET_PAINFUL_PAGE_LOAD, absl::nullopt, &observer);
proto::OPTIMIZATION_TARGET_MODEL_VALIDATION, absl::nullopt, &observer);

histogram_tester.ExpectUniqueSample(
"OptimizationGuide.PredictionManager.ModelAvailableAtRegistration."
"ModelValidation",
false, 1);
}

TEST_F(PredictionManagerTest, UpdatePredictionModelsWithInvalidModel) {
Expand Down Expand Up @@ -931,6 +952,16 @@ TEST_F(PredictionManagerTest, UpdateModelForUnregisteredTargetOnModelReady) {

histogram_tester.ExpectTotalCount(
"OptimizationGuide.PredictionModelLoadedVersion.PainfulPageLoad", 0);

// Now register the model.
FakeOptimizationTargetModelObserver observer;
prediction_manager()->AddObserverForOptimizationTargetModel(
proto::OPTIMIZATION_TARGET_PAINFUL_PAGE_LOAD, absl::nullopt, &observer);

histogram_tester.ExpectUniqueSample(
"OptimizationGuide.PredictionManager.ModelAvailableAtRegistration."
"PainfulPageLoad",
true, 1);
}

TEST_F(PredictionManagerTest,
Expand All @@ -952,6 +983,10 @@ TEST_F(PredictionManagerTest,
EXPECT_FALSE(prediction_model_fetcher()->models_fetched());
histogram_tester.ExpectUniqueSample(
"OptimizationGuide.PredictionModelLoadedVersion.PainfulPageLoad", 1, 1);
histogram_tester.ExpectUniqueSample(
"OptimizationGuide.PredictionManager.ModelAvailableAtRegistration."
"PainfulPageLoad",
true, 1);
}

TEST_F(PredictionManagerTest,
Expand All @@ -975,6 +1010,10 @@ TEST_F(PredictionManagerTest,
EXPECT_FALSE(prediction_model_fetcher()->models_fetched());
histogram_tester.ExpectUniqueSample(
"OptimizationGuide.PredictionModelLoadedVersion.PainfulPageLoad", 1, 1);
histogram_tester.ExpectUniqueSample(
"OptimizationGuide.PredictionManager.ModelAvailableAtRegistration."
"PainfulPageLoad",
true, 1);
}

TEST_F(PredictionManagerTest, ModelFetcherTimerRetryDelay) {
Expand Down
14 changes: 14 additions & 0 deletions tools/metrics/histograms/metadata/optimization/histograms.xml
Original file line number Diff line number Diff line change
Expand Up @@ -823,6 +823,20 @@ chromium-metrics-reviews@google.com.
<token key="OptimizationTarget" variants="OptimizationTarget"/>
</histogram>

<histogram base="true"
name="OptimizationGuide.PredictionManager.ModelAvailableAtRegistration.{OptimizationTarget}"
units="BooleanAvailable" expires_after="M106">
<owner>sophiechang@chromium.org</owner>
<owner>mcrouse@chromium.org</owner>
<summary>
Records whether the model was in the local store for {OptimizationTarget}
when the observer for {OptimizationTarget} was registered with the
prediction manager. Recorded at most once per session (profile start) when
{OptimizationTarget} is registered.
</summary>
<token key="OptimizationTarget" variants="OptimizationTarget"/>
</histogram>

<histogram base="true"
name="OptimizationGuide.PredictionManager.ModelTypeChanged.{OptimizationTarget}"
units="BooleanChanged" expires_after="M106">
Expand Down

0 comments on commit 82c6352

Please sign in to comment.