Skip to content

Commit

Permalink
Revert "[iOS] Delete GN flags for mach absolute time ticks"
Browse files Browse the repository at this point in the history
This reverts commit 734471f.

Reason for revert:
LUCI Bisection identified this CL as the culprit of a build failure. See the analysis: https://luci-bisection.appspot.com/analysis/b/8770136478150458481

Sample failed build: https://ci.chromium.org/b/8770136478150458481

If this is a false positive, please report it at https://bugs.chromium.org/p/chromium/issues/entry?comment=Analysis%3A+https%3A%2F%2Fluci-bisection.appspot.com%2Fanalysis%2Fb%2F8770136478150458481&components=Tools%3ETest%3EFindit&labels=LUCI-Bisection-Wrong%2CPri-3%2CType-Bug&status=Available&summary=Wrongly+blamed+https%3A%2F%2Fchromium-review.googlesource.com%2Fc%2Fchromium%2Fsrc%2F%2B%2F4850577

Original change's description:
> [iOS] Delete GN flags for mach absolute time ticks
>
> Mach absolute time ticks have been enabled by default for a month
> with no known issues, so this CL cleans up the corresponding GN
> flags.
>
> Bug: 1414153
> Change-Id: I981f7e823295a5f8c9a9d186e356243e7f76eef1
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4850577
> Commit-Queue: Ali Juma <ajuma@chromium.org>
> Reviewed-by: Ian Vollick <vollick@chromium.org>
> Reviewed-by: Mark Mentovai <mark@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#1195534}
>

Bug: 1414153
Change-Id: I9bd6656f514dc19a9c647f9ff845420762c462f4
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4858437
Commit-Queue: luci-bisection@appspot.gserviceaccount.com <luci-bisection@appspot.gserviceaccount.com>
Bot-Commit: luci-bisection@appspot.gserviceaccount.com <luci-bisection@appspot.gserviceaccount.com>
Owners-Override: luci-bisection@appspot.gserviceaccount.com <luci-bisection@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/heads/main@{#1195555}
  • Loading branch information
luci-bisection@appspot.gserviceaccount.com authored and Chromium LUCI CQ committed Sep 12, 2023
1 parent 2ad744f commit 2726be5
Show file tree
Hide file tree
Showing 9 changed files with 131 additions and 1 deletion.
19 changes: 18 additions & 1 deletion base/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,11 @@ if (is_mac) {
import("//third_party/protobuf/proto_library.gni")
}

if (is_apple) {
# Buildflags to control time behavior on iOS in file shared with mac.
import("//base/time/buildflags/buildflags.gni")
}

if (is_win) {
import("//build/config/win/control_flow_guard.gni")
}
Expand Down Expand Up @@ -88,6 +93,14 @@ assert(!is_nacl || is_nacl_saigo,
assert(!is_win || is_clang,
"only clang-cl is supported on Windows, see https://crbug.com/988071")

if (is_apple) {
assert(!use_blink || enable_mach_absolute_time_ticks,
"use_blink requires mach absolute time ticks")

assert(!is_mac || enable_mach_absolute_time_ticks,
"mac requires mach absolute time ticks")
}

# Determines whether libevent should be dep.
dep_libevent = !is_fuchsia && !is_win && !is_mac && !is_nacl

Expand Down Expand Up @@ -1053,6 +1066,10 @@ component("base") {
# to provide the appropriate `#define` here.
defines += [ "IS_RAW_PTR_IMPL" ]

if (is_apple) {
deps += [ "//base/time/buildflags:buildflags" ]
}

if (build_rust_json_reader) {
deps += [ "//third_party/rust/serde_json_lenient/v0_1/wrapper" ]
}
Expand Down Expand Up @@ -3723,7 +3740,7 @@ test("base_unittests") {
]
}

if (is_apple) {
if (is_apple && enable_mach_absolute_time_ticks) {
sources += [ "time/time_apple_unittest.mm" ]
}

Expand Down
7 changes: 7 additions & 0 deletions base/allocator/partition_allocator/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -730,6 +730,13 @@ buildflag_header("partition_alloc_buildflags") {
"ENABLE_PKEYS=$enable_pkeys",
"ENABLE_THREAD_ISOLATION=$enable_pkeys",
]

if (is_apple) {
# TODO(crbug.com/1414153): once TimeTicks::Now behavior is unified on iOS,
# this should be removed.
flags += [ "PARTITION_ALLOC_ENABLE_MACH_ABSOLUTE_TIME_TICKS=" +
"$partition_alloc_enable_mach_absolute_time_ticks" ]
}
}

buildflag_header("chromecast_buildflags") {
Expand Down
14 changes: 14 additions & 0 deletions base/allocator/partition_allocator/partition_alloc.gni
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,12 @@ declare_args() {
# Shadow metadata is still under development and only supports Linux
# for now.
enable_shadow_metadata = false

if (is_apple) {
# TODO(crbug.com/1414153): this should be removed once the use of mach
# absolute time ticks is successfully launched on iOS.
partition_alloc_enable_mach_absolute_time_ticks = true
}
}

# *Scan is currently only used by Chromium, and supports only 64-bit.
Expand Down Expand Up @@ -306,6 +312,14 @@ assert(!use_asan_backup_ref_ptr || is_asan,
assert(!use_asan_unowned_ptr || is_asan,
"AsanUnownedPtr requires AddressSanitizer")

if (is_apple) {
assert(!use_blink || partition_alloc_enable_mach_absolute_time_ticks,
"use_blink requires partition_alloc_enable_mach_absolute_time_ticks")

assert(!is_mac || partition_alloc_enable_mach_absolute_time_ticks,
"mac requires partition_alloc_enable_mach_absolute_time_ticks")
}

# AsanBackupRefPtr is not supported outside Chromium. The implementation is
# entangled with `//base`. The code is only physically located with the rest of
# `raw_ptr` to keep it together.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,9 @@ class PA_COMPONENT_EXPORT(PARTITION_ALLOC) TimeDelta {
static TimeDelta FromZxDuration(zx_duration_t nanos);
#endif
#if BUILDFLAG(IS_APPLE)
#if BUILDFLAG(PARTITION_ALLOC_ENABLE_MACH_ABSOLUTE_TIME_TICKS)
static TimeDelta FromMachTime(uint64_t mach_time);
#endif // BUILDFLAG(PARTITION_ALLOC_ENABLE_MACH_ABSOLUTE_TIME_TICKS)
#endif // BUILDFLAG(IS_APPLE)

// Converts an integer value representing TimeDelta to a class. This is used
Expand Down Expand Up @@ -849,12 +851,14 @@ class PA_COMPONENT_EXPORT(PARTITION_ALLOC) TimeTicks
#endif

#if BUILDFLAG(IS_APPLE)
#if BUILDFLAG(PARTITION_ALLOC_ENABLE_MACH_ABSOLUTE_TIME_TICKS)
static TimeTicks FromMachAbsoluteTime(uint64_t mach_absolute_time);

// Sets the current Mach timebase to `timebase`. Returns the old timebase.
static mach_timebase_info_data_t SetMachTimebaseInfoForTesting(
mach_timebase_info_data_t timebase);

#endif // BUILDFLAG(PARTITION_ALLOC_ENABLE_MACH_ABSOLUTE_TIME_TICKS)
#endif // BUILDFLAG(IS_APPLE)

#if BUILDFLAG(IS_ANDROID) || BUILDFLAG(PA_IS_CHROMEOS_ASH)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@

namespace {

#if BUILDFLAG(PARTITION_ALLOC_ENABLE_MACH_ABSOLUTE_TIME_TICKS)
// Returns a pointer to the initialized Mach timebase info struct.
mach_timebase_info_data_t* MachTimebaseInfo() {
static mach_timebase_info_data_t timebase_info = []() {
Expand Down Expand Up @@ -80,14 +81,29 @@ int64_t MachTimeToMicroseconds(uint64_t mach_time) {
// 9223372036854775807 / (1e6 * 60 * 60 * 24 * 365.2425) = 292,277).
return checked_cast<int64_t>(microseconds);
}
#endif // BUILDFLAG(PARTITION_ALLOC_ENABLE_MACH_ABSOLUTE_TIME_TICKS)

// Returns monotonically growing number of ticks in microseconds since some
// unspecified starting point.
int64_t ComputeCurrentTicks() {
#if !BUILDFLAG(PARTITION_ALLOC_ENABLE_MACH_ABSOLUTE_TIME_TICKS)
struct timespec tp;
// clock_gettime() returns 0 on success and -1 on failure. Failure can only
// happen because of bad arguments (unsupported clock type or timespec
// pointer out of accessible address space). Here it is known that neither
// can happen since the timespec parameter is stack allocated right above and
// `CLOCK_MONOTONIC` is supported on all versions of iOS that Chrome is
// supported on.
int res = clock_gettime(CLOCK_MONOTONIC, &tp);
PA_BASE_DCHECK(0 == res) << "Failed clock_gettime, errno: " << errno;

return (int64_t)tp.tv_sec * 1000000 + tp.tv_nsec / 1000;
#else
// mach_absolute_time is it when it comes to ticks on the Mac. Other calls
// with less precision (such as TickCount) just call through to
// mach_absolute_time.
return MachTimeToMicroseconds(mach_absolute_time());
#endif // !BUILDFLAG(PARTITION_ALLOC_ENABLE_MACH_ABSOLUTE_TIME_TICKS)
}

int64_t ComputeThreadTicks() {
Expand Down Expand Up @@ -172,10 +188,12 @@ Time TimeNowFromSystemTimeIgnoringOverride() {

// TimeDelta ------------------------------------------------------------------

#if BUILDFLAG(PARTITION_ALLOC_ENABLE_MACH_ABSOLUTE_TIME_TICKS)
// static
TimeDelta TimeDelta::FromMachTime(uint64_t mach_time) {
return Microseconds(MachTimeToMicroseconds(mach_time));
}
#endif // BUILDFLAG(PARTITION_ALLOC_ENABLE_MACH_ABSOLUTE_TIME_TICKS)

// TimeTicks ------------------------------------------------------------------

Expand All @@ -195,6 +213,7 @@ TimeTicks TimeTicksNowIgnoringOverride() {
return true;
}

#if BUILDFLAG(PARTITION_ALLOC_ENABLE_MACH_ABSOLUTE_TIME_TICKS)
// static
TimeTicks TimeTicks::FromMachAbsoluteTime(uint64_t mach_absolute_time) {
return TimeTicks(MachTimeToMicroseconds(mach_absolute_time));
Expand All @@ -210,9 +229,15 @@ TimeTicks TimeTicksNowIgnoringOverride() {
return orig_timebase;
}

#endif // BUILDFLAG(PARTITION_ALLOC_ENABLE_MACH_ABSOLUTE_TIME_TICKS)

// static
TimeTicks::Clock TimeTicks::GetClock() {
#if !BUILDFLAG(PARTITION_ALLOC_ENABLE_MACH_ABSOLUTE_TIME_TICKS)
return Clock::IOS_CF_ABSOLUTE_TIME_MINUS_KERN_BOOTTIME;
#else
return Clock::MAC_MACH_ABSOLUTE_TIME;
#endif // !BUILDFLAG(PARTITION_ALLOC_ENABLE_MACH_ABSOLUTE_TIME_TICKS)
}

// ThreadTicks ----------------------------------------------------------------
Expand Down
15 changes: 15 additions & 0 deletions base/time/buildflags/BUILD.gn
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
# Copyright 2023 The Chromium Authors
# Use of this source code is governed by a BSD-style license that can be
# found in the LICENSE file.

import("//build/buildflag_header.gni")
import("buildflags.gni")

# Generate a buildflag header for compile-time checking of mach absolute time
# support in TimeTicks
# TODO(crbug.com/1414153): this should be removed once there is a unified
# approach to TimeTicks::Now on iOS.
buildflag_header("buildflags") {
header = "buildflags.h"
flags = [ "ENABLE_MACH_ABSOLUTE_TIME_TICKS=$enable_mach_absolute_time_ticks" ]
}
13 changes: 13 additions & 0 deletions base/time/buildflags/buildflags.gni
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
# Copyright 2023 The Chromium Authors
# Use of this source code is governed by a BSD-style license that can be
# found in the LICENSE file.

import("//build/config/features.gni")

# TODO(crbug.com/1414153): this should be removed once the use of mach absolute
# time ticks is successfully launched on iOS.
declare_args() {
# use_blink currently assumes mach absolute time ticks (eg, to ensure that
# trace events cohere).
enable_mach_absolute_time_ticks = true
}
4 changes: 4 additions & 0 deletions base/time/time.h
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,9 @@ class BASE_EXPORT TimeDelta {
static TimeDelta FromZxDuration(zx_duration_t nanos);
#endif
#if BUILDFLAG(IS_APPLE)
#if BUILDFLAG(ENABLE_MACH_ABSOLUTE_TIME_TICKS)
static TimeDelta FromMachTime(uint64_t mach_time);
#endif // BUILDFLAG(ENABLE_MACH_ABSOLUTE_TIME_TICKS)
#endif // BUILDFLAG(IS_APPLE)

// Converts an integer value representing TimeDelta to a class. This is used
Expand Down Expand Up @@ -1196,12 +1198,14 @@ class BASE_EXPORT TimeTicks : public time_internal::TimeBase<TimeTicks> {
#endif

#if BUILDFLAG(IS_APPLE)
#if BUILDFLAG(ENABLE_MACH_ABSOLUTE_TIME_TICKS)
static TimeTicks FromMachAbsoluteTime(uint64_t mach_absolute_time);

// Sets the current Mach timebase to `timebase`. Returns the old timebase.
static mach_timebase_info_data_t SetMachTimebaseInfoForTesting(
mach_timebase_info_data_t timebase);

#endif // BUILDFLAG(ENABLE_MACH_ABSOLUTE_TIME_TICKS)
#endif // BUILDFLAG(IS_APPLE)

#if BUILDFLAG(IS_ANDROID) || BUILDFLAG(IS_CHROMEOS_ASH)
Expand Down
31 changes: 31 additions & 0 deletions base/time/time_apple.mm
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,15 @@
#include "base/time/time_override.h"
#include "build/build_config.h"

#if !BUILDFLAG(ENABLE_MACH_ABSOLUTE_TIME_TICKS)
#include <errno.h>
#include <time.h>
#include "base/ios/ios_util.h"
#endif // !BUILDFLAG(ENABLE_MACH_ABSOLUTE_TIME_TICKS)

namespace {

#if BUILDFLAG(ENABLE_MACH_ABSOLUTE_TIME_TICKS)
// Returns a pointer to the initialized Mach timebase info struct.
mach_timebase_info_data_t* MachTimebaseInfo() {
static mach_timebase_info_data_t timebase_info = []() {
Expand Down Expand Up @@ -77,14 +84,29 @@ int64_t MachTimeToMicroseconds(uint64_t mach_time) {
// 9223372036854775807 / (1e6 * 60 * 60 * 24 * 365.2425) = 292,277).
return base::checked_cast<int64_t>(microseconds);
}
#endif // BUILDFLAG(ENABLE_MACH_ABSOLUTE_TIME_TICKS)

// Returns monotonically growing number of ticks in microseconds since some
// unspecified starting point.
int64_t ComputeCurrentTicks() {
#if !BUILDFLAG(ENABLE_MACH_ABSOLUTE_TIME_TICKS)
struct timespec tp;
// clock_gettime() returns 0 on success and -1 on failure. Failure can only
// happen because of bad arguments (unsupported clock type or timespec pointer
// out of accessible address space). Here it is known that neither can happen
// since the timespec parameter is stack allocated right above and
// `CLOCK_MONOTONIC` is supported on all versions of iOS that Chrome is
// supported on.
int res = clock_gettime(CLOCK_MONOTONIC, &tp);
DCHECK_EQ(res, 0) << "Failed clock_gettime, errno: " << errno;

return (int64_t)tp.tv_sec * 1000000 + tp.tv_nsec / 1000;
#else
// mach_absolute_time is it when it comes to ticks on the Mac. Other calls
// with less precision (such as TickCount) just call through to
// mach_absolute_time.
return MachTimeToMicroseconds(mach_absolute_time());
#endif // !BUILDFLAG(ENABLE_MACH_ABSOLUTE_TIME_TICKS)
}

int64_t ComputeThreadTicks() {
Expand Down Expand Up @@ -172,10 +194,12 @@ Time TimeNowFromSystemTimeIgnoringOverride() {

// TimeDelta ------------------------------------------------------------------

#if BUILDFLAG(ENABLE_MACH_ABSOLUTE_TIME_TICKS)
// static
TimeDelta TimeDelta::FromMachTime(uint64_t mach_time) {
return Microseconds(MachTimeToMicroseconds(mach_time));
}
#endif // BUILDFLAG(ENABLE_MACH_ABSOLUTE_TIME_TICKS)

// TimeTicks ------------------------------------------------------------------

Expand All @@ -195,6 +219,7 @@ TimeTicks TimeTicksNowIgnoringOverride() {
return true;
}

#if BUILDFLAG(ENABLE_MACH_ABSOLUTE_TIME_TICKS)
// static
TimeTicks TimeTicks::FromMachAbsoluteTime(uint64_t mach_absolute_time) {
return TimeTicks(MachTimeToMicroseconds(mach_absolute_time));
Expand All @@ -210,9 +235,15 @@ TimeTicks TimeTicksNowIgnoringOverride() {
return orig_timebase;
}

#endif // BUILDFLAG(ENABLE_MACH_ABSOLUTE_TIME_TICKS)

// static
TimeTicks::Clock TimeTicks::GetClock() {
#if !BUILDFLAG(ENABLE_MACH_ABSOLUTE_TIME_TICKS)
return Clock::IOS_CF_ABSOLUTE_TIME_MINUS_KERN_BOOTTIME;
#else
return Clock::MAC_MACH_ABSOLUTE_TIME;
#endif // !BUILDFLAG(ENABLE_MACH_ABSOLUTE_TIME_TICKS)
}

// ThreadTicks ----------------------------------------------------------------
Expand Down

0 comments on commit 2726be5

Please sign in to comment.