Skip to content

Commit 0baf6e4

Browse files
Anthony Vallee-Duboiscopybara-github
authored andcommitted
Add an "Excluded b/c cmd line is not ready" group to qpc trial
This is to help diagnose why all clients in the trial end up in the Control group Bug: 410560675 Change-Id: Iaf6a368763d2d440a6ad07c75ed7f17088742d93 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6525906 Commit-Queue: Anthony Vallée-Dubois <[email protected]> Reviewed-by: Joe Mason <[email protected]> Cr-Commit-Position: refs/heads/main@{#1458877} NOKEYCHECK=True GitOrigin-RevId: 2d2e886b7680dd9320584ff069117fdfada32a32
1 parent a5d130a commit 0baf6e4

File tree

1 file changed

+25
-1
lines changed

1 file changed

+25
-1
lines changed

time/time_win.cc

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -571,6 +571,8 @@ TimeTicks QPCNow() {
571571
return TimeTicks() + QPCValueToTimeDelta(QPCNowRaw());
572572
}
573573

574+
std::atomic<bool> g_opted_out_of_qpc_trial_because_no_command_line = false;
575+
574576
void InitializeNowFunctionPointer() {
575577
LARGE_INTEGER ticks_per_sec = {};
576578
// `QueryPerformanceFrequency` always succeeds and sets its out parameter to a
@@ -595,6 +597,11 @@ void InitializeNowFunctionPointer() {
595597
// ~97% of users fall within this category.
596598
bool eligible_for_high_res_time_ticks = false;
597599

600+
// To debug an issue where in the field, all clients are in the Control group.
601+
// We suspect it might be due to the command line not being ready before this
602+
// function is called in some build configurations.
603+
bool opted_out_because_no_command_line = false;
604+
598605
// `ticks_per_sec.QuadPart <= 0` shouldn't happen post-WinXP (see CHECKs
599606
// above) but if it does, QPC is broken and shouldn't be used for any reason.
600607
if (ticks_per_sec.QuadPart > 0) {
@@ -633,6 +640,8 @@ void InitializeNowFunctionPointer() {
633640
// TimeTicks so the dice roll will not be used.
634641
force_high_res_time_ticks = base::RandDouble() < 0.5;
635642
}
643+
} else {
644+
opted_out_because_no_command_line = true;
636645
}
637646

638647
eligible_for_high_res_time_ticks =
@@ -674,6 +683,11 @@ void InitializeNowFunctionPointer() {
674683
// via TimeTicksNowIgnoringOverride() for future calls to TimeTicks::Now().
675684
internal::g_time_ticks_now_function.store(now_function,
676685
std::memory_order_relaxed);
686+
687+
// Only the thread setting the functions should report whether its command
688+
// line was ready.
689+
g_opted_out_of_qpc_trial_because_no_command_line.store(
690+
opted_out_because_no_command_line, std::memory_order_relaxed);
677691
}
678692
}
679693

@@ -688,6 +702,7 @@ enum class HighResolutionTrialState {
688702
kExcludedFromTrial,
689703
kDontUseHighResolution,
690704
kUseHighResolution,
705+
kExcludedBecauseNoCommandLine,
691706
};
692707

693708
HighResolutionTrialState GetHighResolutionTrialState() {
@@ -706,7 +721,12 @@ HighResolutionTrialState GetHighResolutionTrialState() {
706721

707722
CPU cpu;
708723
if (!cpu.has_non_stop_time_stamp_counter()) {
709-
if (is_high_res) {
724+
if (g_opted_out_of_qpc_trial_because_no_command_line.load(
725+
std::memory_order_relaxed)) {
726+
// If there was no command line ready when initializing the time
727+
// functions, put the client in a separate group.
728+
return HighResolutionTrialState::kExcludedBecauseNoCommandLine;
729+
} else if (is_high_res) {
710730
// If the device isn't considered eligible for QPC-based TimeTicks but is
711731
// using it regardless, it means that it's part of the experimental QPC
712732
// group.
@@ -878,6 +898,9 @@ bool TimeTicks::GetHighResolutionTimeTicksFieldTrial(std::string* trial_name,
878898
case HighResolutionTrialState::kUseHighResolution:
879899
*group_name = "Enabled";
880900
break;
901+
case HighResolutionTrialState::kExcludedBecauseNoCommandLine:
902+
*group_name = "ExcludedBecauseNoCommandLine";
903+
break;
881904
}
882905

883906
*trial_name = "HighResolutionTimeTicks";
@@ -895,6 +918,7 @@ void TimeTicks::MaybeAddHighResolutionTimeTicksSwitch(
895918
// TimeTicks, don't pass any command line flag.
896919
break;
897920
case HighResolutionTrialState::kExcludedFromTrial:
921+
case HighResolutionTrialState::kExcludedBecauseNoCommandLine:
898922
// In the cases of "Control" and "Excluded", tell the child process not to
899923
// use QPC for TimeTicks to match the browser process.
900924
[[fallthrough]];

0 commit comments

Comments
 (0)