From a5b69b877db7d39d93add0e6065c20a30ea5cd9d Mon Sep 17 00:00:00 2001 From: Chris Kennelly Date: Fri, 17 Jul 2026 20:23:52 -0700 Subject: [PATCH] Fix ClockTest.ClockTicks flakiness in stats_test by introducing a GetTimePair() helper to read absl::Now() and CycleClock::Now() in a loop without preemption. PiperOrigin-RevId: 949893104 --- .bazelrc | 14 ++++++++----- .github/workflows/ci.yml | 2 +- MODULE.bazel | 6 ------ ci/BUILD | 25 +++++----------------- ci/docker_toolchain/BUILD | 21 ------------------- ci/docker_toolchain/MODULE.bazel | 19 ----------------- tcmalloc/stats_test.cc | 36 +++++++++++++++++++++++++------- 7 files changed, 43 insertions(+), 80 deletions(-) delete mode 100644 ci/docker_toolchain/BUILD delete mode 100644 ci/docker_toolchain/MODULE.bazel diff --git a/.bazelrc b/.bazelrc index 1658351e8..f7432e5ba 100644 --- a/.bazelrc +++ b/.bazelrc @@ -45,11 +45,15 @@ build:asan-libfuzzer --@rules_fuzzing//fuzzing:cc_engine=@rules_fuzzing//fuzzing build:asan-libfuzzer --@rules_fuzzing//fuzzing:cc_engine_instrumentation=libfuzzer build:asan-libfuzzer --@rules_fuzzing//fuzzing:cc_engine_sanitizer=asan -# Configure our Docker toolchain setup. -build --extra_toolchains=@docker_toolchain//:all -build:libstdc++ --features=libstdc++ -build:gcc --platforms=//ci:linux_gcc -build:clang --platforms=//ci:linux_clang +# TODO(b/530366451) Replace these with a proper toolchain setup +build:libstdc++ --repo_env=BAZEL_CXXOPTS= --repo_env=BAZEL_LINKOPTS= --repo_env=CPLUS_INCLUDE_PATH= +build:libstdc++ --action_env=BAZEL_CXXOPTS= --action_env=BAZEL_LINKOPTS= --action_env=CPLUS_INCLUDE_PATH= +build:libstdc++ --host_action_env=BAZEL_CXXOPTS= --host_action_env=BAZEL_LINKOPTS= --host_action_env=CPLUS_INCLUDE_PATH= + +# TODO(b/530366451) This uses gcc 14, replace with a more recent version. +build:gcc --config=libstdc++ +build:gcc --host_action_env=CC=/usr/local/bin/gcc --action_env=CC=/usr/bin/gcc +build:gcc --host_action_env=CXX=/usr/local/bin/g++ --action_env=CXX=/usr/bin/g++ # Remote build configuration for CI. build:remote-base --define=EXECUTOR=remote diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 8c7654350..305bf50e2 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -45,7 +45,7 @@ jobs: runs-on: ubuntu-24.04 name: Linux ${{ matrix.toolchain }} ${{ matrix.compilation_mode }} ${{ matrix.exceptions }} Bazel container: - image: gcr.io/google.com/absl-177019/gloop-linux-toolchain-latest@sha256:a820c79d3f356a64db121a4ee16c4408df18f9d2c8bf6c6abfae21666ac003ba + image: gcr.io/google.com/absl-177019/gloop-linux-latest@sha256:903f3d7733dd615bc1542b548fab4b649cf6e48118251a2c6527ce4e30543c6b options: --cap-add=SYS_PTRACE credentials: # TODO(b/530366451) Switch to Workload Identity Federation to avoid the need for secrets and diff --git a/MODULE.bazel b/MODULE.bazel index 6113e7df5..46fb5d59a 100644 --- a/MODULE.bazel +++ b/MODULE.bazel @@ -38,9 +38,3 @@ bazel_dep( version = "20260708.rc1", repo_name = "do_not_use_for_gloop_visibility_only", ) - -bazel_dep(name = "docker_toolchain", version = "0.1", dev_dependency = True) -local_path_override( - module_name = "docker_toolchain", - path = "ci/docker_toolchain", -) diff --git a/ci/BUILD b/ci/BUILD index 8aa759c13..2d87c20bb 100644 --- a/ci/BUILD +++ b/ci/BUILD @@ -1,4 +1,7 @@ -package(default_visibility = ["//visibility:private"]) +package( + default_applicable_licenses = ["//tcmalloc:license"], + default_visibility = ["//visibility:private"], +) platform( name = "rbe", @@ -7,27 +10,9 @@ platform( "@platforms//cpu:x86_64", ], exec_properties = { - "container-image": "docker://gcr.io/google.com/absl-177019/gloop-linux-latest@sha256:e199623ad0b61b30d240e8bbddf473f65cb1a11da09462001d83b5066dbd9e98", + "container-image": "docker://gcr.io/google.com/absl-177019/gloop-linux-latest@sha256:903f3d7733dd615bc1542b548fab4b649cf6e48118251a2c6527ce4e30543c6b", "OSArch": "amd64", "dockerRunAsRoot": "true", "cache-silo-key": "n2-highmem-8-2026-07-07", }, ) - -platform( - name = "linux_gcc", - constraint_values = [ - "@platforms//os:linux", - "@platforms//cpu:x86_64", - "@bazel_tools//tools/cpp:gcc", - ], -) - -platform( - name = "linux_clang", - constraint_values = [ - "@platforms//os:linux", - "@platforms//cpu:x86_64", - "@bazel_tools//tools/cpp:clang", - ], -) diff --git a/ci/docker_toolchain/BUILD b/ci/docker_toolchain/BUILD deleted file mode 100644 index ee078f0c5..000000000 --- a/ci/docker_toolchain/BUILD +++ /dev/null @@ -1,21 +0,0 @@ -# Copyright 2026 The TCMalloc Authors -# -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# https://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. - -# Standalone fallback toolchain module when building outside the Docker container. -# Inside the container, this module is overridden via /etc/bazel.bazelrc to point to an actual -# toolchain. - -package(default_visibility = ["//visibility:public"]) - -filegroup(name = "empty") diff --git a/ci/docker_toolchain/MODULE.bazel b/ci/docker_toolchain/MODULE.bazel deleted file mode 100644 index 1a3a943ee..000000000 --- a/ci/docker_toolchain/MODULE.bazel +++ /dev/null @@ -1,19 +0,0 @@ -# Copyright 2026 The TCMalloc Authors -# -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# https://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. - -module( - name = "docker_toolchain", - version = "0.1", -) - diff --git a/tcmalloc/stats_test.cc b/tcmalloc/stats_test.cc index 91dd96380..c61f174fc 100644 --- a/tcmalloc/stats_test.cc +++ b/tcmalloc/stats_test.cc @@ -150,22 +150,42 @@ TEST(PageAllocInfo, Large) { EXPECT_EQ(slack, info.slack()); } +struct TimePair { + absl::Time time; + double cycles_seconds; +}; + +TimePair GetTimePair() { + TimePair best; + absl::Duration best_diff = absl::InfiniteDuration(); + for (int i = 0; i < 100; ++i) { + absl::Time t0 = absl::Now(); + int64_t cycles = absl::base_internal::CycleClock::Now(); + absl::Time t1 = absl::Now(); + absl::Duration diff = t1 - t0; + if (diff < best_diff) { + best_diff = diff; + best.time = t0 + diff / 2; + best.cycles_seconds = static_cast(cycles) / + absl::base_internal::CycleClock::Frequency(); + } + } + return best; +} + TEST(ClockTest, ClockTicks) { // It's a bit ironic to test this clock against other clocks since // this exists because we don't trust other clocks. But hopefully // no one is using libfaketime on this binary, and of course we // don't care about signal safety, just ticking. - const absl::Time before = absl::Now(); - const double b = absl::base_internal::CycleClock::Now() / - absl::base_internal::CycleClock::Frequency(); + const TimePair before = GetTimePair(); static const absl::Duration kDur = absl::Milliseconds(500); absl::SleepFor(kDur); - const double a = absl::base_internal::CycleClock::Now() / - absl::base_internal::CycleClock::Frequency(); - const absl::Time after = absl::Now(); + const TimePair after = GetTimePair(); - const absl::Duration actual = (after - before); - const absl::Duration measured = absl::Seconds(a - b); + const absl::Duration actual = (after.time - before.time); + const absl::Duration measured = + absl::Seconds(after.cycles_seconds - before.cycles_seconds); EXPECT_LE(actual * 0.99, measured) << actual; EXPECT_GE(actual * 1.01, measured) << actual; }