Commit 921cb3f0 authored by Sophie Chang's avatar Sophie Chang Committed by Commit Bot

Add feature param for painful page load metrics-only collection

This will basically only affect painful page loads since we would taint
the data but made the method more general since logic is in the keyed
service.

Bug: 1001194
Change-Id: Ib19113a08251a17346697ef860a0270d5d8f6457
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1856439
Commit-Queue: Sophie Chang <sophiechang@chromium.org>
Reviewed-by: default avatarMichael Crouse <mcrouse@chromium.org>
Reviewed-by: default avatarTarun Bansal <tbansal@chromium.org>
Cr-Commit-Position: refs/heads/master@{#708589}
parent 1f351119
...@@ -99,20 +99,21 @@ optimization_guide::OptimizationGuideDecision ResolveOptimizationGuideDecision( ...@@ -99,20 +99,21 @@ optimization_guide::OptimizationGuideDecision ResolveOptimizationGuideDecision(
optimization_guide::OptimizationTypeDecision optimization_type_decision) { optimization_guide::OptimizationTypeDecision optimization_type_decision) {
switch (optimization_target_decision) { switch (optimization_target_decision) {
case optimization_guide::OptimizationTargetDecision::kPageLoadDoesNotMatch: case optimization_guide::OptimizationTargetDecision::kPageLoadDoesNotMatch:
case optimization_guide::OptimizationTargetDecision::
kModelNotAvailableOnClient:
case optimization_guide::OptimizationTargetDecision::
kModelPredictionHoldback:
return optimization_guide::OptimizationGuideDecision::kFalse; return optimization_guide::OptimizationGuideDecision::kFalse;
case optimization_guide::OptimizationTargetDecision::kPageLoadMatches: case optimization_guide::OptimizationTargetDecision::kPageLoadMatches:
return GetOptimizationGuideDecisionFromOptimizationTypeDecision( return GetOptimizationGuideDecisionFromOptimizationTypeDecision(
optimization_type_decision); optimization_type_decision);
case optimization_guide::OptimizationTargetDecision::
kModelNotAvailableOnClient:
return optimization_guide::OptimizationGuideDecision::kFalse;
default: default:
return optimization_guide::OptimizationGuideDecision::kUnknown; return optimization_guide::OptimizationGuideDecision::kUnknown;
} }
static_assert( static_assert(
optimization_guide::OptimizationTargetDecision::kMaxValue == optimization_guide::OptimizationTargetDecision::kMaxValue ==
optimization_guide::OptimizationTargetDecision:: optimization_guide::OptimizationTargetDecision::
kModelNotAvailableOnClient, kModelPredictionHoldback,
"This function should be updated when a new OptimizationTargetDecision " "This function should be updated when a new OptimizationTargetDecision "
"is added"); "is added");
} }
...@@ -190,8 +191,12 @@ OptimizationGuideKeyedService::CanApplyOptimization( ...@@ -190,8 +191,12 @@ OptimizationGuideKeyedService::CanApplyOptimization(
if (prediction_manager_) { if (prediction_manager_) {
optimization_target_decision = prediction_manager_->ShouldTargetNavigation( optimization_target_decision = prediction_manager_->ShouldTargetNavigation(
navigation_handle, optimization_target); navigation_handle, optimization_target);
// TODO(crbug/1001194): Add feature param for propagating decision that if (optimization_guide::features::
// allows for us to collect metrics without tainting the data. ShouldOverrideOptimizationTargetDecisionForMetricsPurposes(
optimization_target)) {
optimization_target_decision = optimization_guide::
OptimizationTargetDecision::kModelPredictionHoldback;
}
} }
LogDecisions(navigation_handle, optimization_target, LogDecisions(navigation_handle, optimization_target,
......
...@@ -730,3 +730,44 @@ IN_PROC_BROWSER_TEST_F( ...@@ -730,3 +730,44 @@ IN_PROC_BROWSER_TEST_F(
kModelNotAvailableOnClient), kModelNotAvailableOnClient),
1); 1);
} }
class OptimizationGuideKeyedServiceModelPredictionHoldbackBrowserTest
: public OptimizationGuideKeyedServiceBrowserTest {
public:
OptimizationGuideKeyedServiceModelPredictionHoldbackBrowserTest() {
scoped_feature_list_.InitAndEnableFeatureWithParameters(
optimization_guide::features::kOptimizationTargetPrediction,
{{"painful_page_load_metrics_only", "true"}});
}
~OptimizationGuideKeyedServiceModelPredictionHoldbackBrowserTest() override =
default;
private:
base::test::ScopedFeatureList scoped_feature_list_;
};
IN_PROC_BROWSER_TEST_F(
OptimizationGuideKeyedServiceModelPredictionHoldbackBrowserTest,
ModelPredictionHoldbackOverridesActualTargetDecision) {
PushHintsComponentAndWaitForCompletion();
RegisterWithKeyedService();
ukm::TestAutoSetUkmRecorder ukm_recorder;
base::HistogramTester histogram_tester;
ui_test_utils::NavigateToURL(browser(), url_with_hints());
EXPECT_EQ(RetryForHistogramUntilCountReached(
histogram_tester, "OptimizationGuide.LoadedHint.Result", 1),
1);
// There should be a hint that matches this URL.
histogram_tester.ExpectUniqueSample("OptimizationGuide.LoadedHint.Result",
true, 1);
EXPECT_EQ(optimization_guide::OptimizationGuideDecision::kFalse,
last_consumer_decision());
histogram_tester.ExpectUniqueSample(
"OptimizationGuide.TargetDecision.PainfulPageLoad",
static_cast<int>(optimization_guide::OptimizationTargetDecision::
kModelPredictionHoldback),
1);
}
...@@ -55,8 +55,12 @@ enum class OptimizationTargetDecision { ...@@ -55,8 +55,12 @@ enum class OptimizationTargetDecision {
// The model needed to make the target decision was not available on the // The model needed to make the target decision was not available on the
// client. // client.
kModelNotAvailableOnClient, kModelNotAvailableOnClient,
// The page load is part of a model prediction holdback where all decisions
// will return |OptimizationGuideDecision::kFalse| in an attempt to not taint
// the data for understanding the production recall of the model.
kModelPredictionHoldback,
// Add new values above this line. // Add new values above this line.
kMaxValue = kModelNotAvailableOnClient, kMaxValue = kModelPredictionHoldback,
}; };
} // namespace optimization_guide } // namespace optimization_guide
......
...@@ -169,5 +169,14 @@ bool IsOptimizationTargetPredictionEnabled() { ...@@ -169,5 +169,14 @@ bool IsOptimizationTargetPredictionEnabled() {
return base::FeatureList::IsEnabled(kOptimizationTargetPrediction); return base::FeatureList::IsEnabled(kOptimizationTargetPrediction);
} }
bool ShouldOverrideOptimizationTargetDecisionForMetricsPurposes(
proto::OptimizationTarget optimization_target) {
if (optimization_target != proto::OPTIMIZATION_TARGET_PAINFUL_PAGE_LOAD)
return false;
return base::GetFieldTrialParamByFeatureAsBool(
kOptimizationTargetPrediction, "painful_page_load_metrics_only", false);
}
} // namespace features } // namespace features
} // namespace optimization_guide } // namespace optimization_guide
...@@ -11,6 +11,7 @@ ...@@ -11,6 +11,7 @@
#include "base/feature_list.h" #include "base/feature_list.h"
#include "base/optional.h" #include "base/optional.h"
#include "base/time/time.h" #include "base/time/time.h"
#include "components/optimization_guide/proto/models.pb.h"
#include "net/nqe/effective_connection_type.h" #include "net/nqe/effective_connection_type.h"
#include "url/gurl.h" #include "url/gurl.h"
...@@ -90,6 +91,12 @@ base::TimeDelta GetHintsFetchRefreshDuration(); ...@@ -90,6 +91,12 @@ base::TimeDelta GetHintsFetchRefreshDuration();
// Returns true if optimization target prediction is enabled. // Returns true if optimization target prediction is enabled.
bool IsOptimizationTargetPredictionEnabled(); bool IsOptimizationTargetPredictionEnabled();
// Returns true if the optimization target decision for |optimization_target|
// should not be propagated to the caller in an effort to fully understand the
// statistics for the served model and not taint the resulting data.
bool ShouldOverrideOptimizationTargetDecisionForMetricsPurposes(
proto::OptimizationTarget optimization_target);
} // namespace features } // namespace features
} // namespace optimization_guide } // namespace optimization_guide
......
...@@ -46140,6 +46140,11 @@ Called by update_net_trust_anchors.py.--> ...@@ -46140,6 +46140,11 @@ Called by update_net_trust_anchors.py.-->
<int value="3" label="Model for target not available on client"> <int value="3" label="Model for target not available on client">
The model for the optimization target not available on the client. The model for the optimization target not available on the client.
</int> </int>
<int value="4" label="In model prediction holdback">
The page load is part of a model prediction holdback where all decisions
will return false in an attempt to not taint the data in order to understand
the production recall of the model.
</int>
</enum> </enum>
<enum name="OptimizationGuideOptimizationTypeDecision"> <enum name="OptimizationGuideOptimizationTypeDecision">
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment