Skip to content

Commit fcf3bfa

Browse files
al45tairal45tair
committed
[Threading] Always round up when converting std::chrono::durations.
We were inadvertently rounding down, which was fine almost 100% of the time, but on Windows where the timer resolution is only 1ms, it made the test _very slightly_ flaky because we were asking to wait for 9ms rather than 10ms, then checking we'd waited at least 10ms. We should explicitly round up, for that reason. rdar://100236038
1 parent 0dc768a commit fcf3bfa

File tree

6 files changed

+97
-20
lines changed

6 files changed

+97
-20
lines changed

‎include/swift/Threading/Impl/C11.h

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,11 +18,12 @@
1818
#define SWIFT_THREADING_IMPL_C11_H
1919

2020
#include <atomic>
21-
#include <chrono>
2221
#include <cstdint>
2322

2423
#include <threads.h>
2524

25+
#include "chrono.h"
26+
2627
#include "llvm/ADT/Optional.h"
2728

2829
#include "swift/Threading/Errors.h"
@@ -180,12 +181,14 @@ inline void cond_wait(cond_handle &handle) {
180181
template <class Rep, class Period>
181182
inline bool cond_wait(cond_handle &handle,
182183
std::chrono::duration<Rep, Period> duration) {
183-
auto deadline = std::chrono::system_clock::now() + duration;
184+
auto to_wait = chrono_utils::ceil<
185+
std::chrono::system_clock::duration>(duration);
186+
auto deadline = std::chrono::system_clock::now() + to_wait;
184187
return cond_wait(handle, deadline);
185188
}
186189
inline bool cond_wait(cond_handle &handle,
187190
std::chrono::system_clock::time_point deadline) {
188-
auto ns = std::chrono::duration_cast<std::chrono::nanoseconds>(
191+
auto ns = chrono_utils::ceil<std::chrono::nanoseconds>(
189192
deadline.time_since_epoch()).count();
190193
struct ::timespec ts = { ::time_t(ns / 1000000000), long(ns % 1000000000) };
191194
SWIFT_C11THREADS_RETURN_TRUE_OR_FALSE(

‎include/swift/Threading/Impl/Darwin.h

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@
2121
#include <os/lock.h>
2222
#include <pthread.h>
2323

24-
#include <chrono>
24+
#include "chrono.h"
2525

2626
#include "llvm/ADT/Optional.h"
2727

@@ -157,14 +157,14 @@ inline void cond_wait(cond_handle &handle) {
157157
template <class Rep, class Period>
158158
inline bool cond_wait(cond_handle &handle,
159159
std::chrono::duration<Rep, Period> duration) {
160-
auto deadline = std::chrono::time_point_cast<
161-
std::chrono::system_clock::duration>(std::chrono::system_clock::now()
162-
+ duration);
160+
auto to_wait = chrono_utils::ceil<
161+
std::chrono::system_clock::duration>(duration);
162+
auto deadline = std::chrono::system_clock::now() + to_wait;
163163
return cond_wait(handle, deadline);
164164
}
165165
inline bool cond_wait(cond_handle &handle,
166166
std::chrono::system_clock::time_point deadline) {
167-
auto ns = std::chrono::duration_cast<std::chrono::nanoseconds>(
167+
auto ns = chrono_utils::ceil<std::chrono::nanoseconds>(
168168
deadline.time_since_epoch()).count();
169169
struct ::timespec ts = { ::time_t(ns / 1000000000), long(ns % 1000000000) };
170170
SWIFT_PTHREADS_RETURN_TRUE_OR_FALSE(

‎include/swift/Threading/Impl/Linux.h

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -21,9 +21,10 @@
2121
#include <pthread.h>
2222

2323
#include <atomic>
24-
#include <chrono>
2524
#include <optional>
2625

26+
#include "chrono.h"
27+
2728
#include "llvm/ADT/Optional.h"
2829

2930
#include "swift/Threading/Errors.h"
@@ -167,14 +168,14 @@ inline void cond_wait(cond_handle &handle) {
167168
template <class Rep, class Period>
168169
inline bool cond_wait(cond_handle &handle,
169170
std::chrono::duration<Rep, Period> duration) {
170-
auto deadline = std::chrono::time_point_cast<
171-
std::chrono::system_clock::duration>(std::chrono::system_clock::now()
172-
+ duration);
171+
auto to_wait = chrono_utils::ceil<
172+
std::chrono::system_clock::duration>(duration);
173+
auto deadline = std::chrono::system_clock::now() + to_wait;
173174
return cond_wait(handle, deadline);
174175
}
175176
inline bool cond_wait(cond_handle &handle,
176177
std::chrono::system_clock::time_point deadline) {
177-
auto ns = std::chrono::duration_cast<std::chrono::nanoseconds>(
178+
auto ns = chrono_utils::ceil<std::chrono::nanoseconds>(
178179
deadline.time_since_epoch()).count();
179180
struct ::timespec ts = { ::time_t(ns / 1000000000), long(ns % 1000000000) };
180181
SWIFT_LINUXTHREADS_RETURN_TRUE_OR_FALSE(

‎include/swift/Threading/Impl/Pthreads.h

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -21,9 +21,10 @@
2121
#include <pthread.h>
2222

2323
#include <atomic>
24-
#include <chrono>
2524
#include <cstdint>
2625

26+
#include "chrono.h"
27+
2728
#include "llvm/ADT/Optional.h"
2829

2930
#include "swift/Threading/Errors.h"
@@ -163,14 +164,14 @@ inline void cond_wait(cond_handle &handle) {
163164
template <class Rep, class Period>
164165
inline bool cond_wait(cond_handle &handle,
165166
std::chrono::duration<Rep, Period> duration) {
166-
auto deadline = std::chrono::time_point_cast<
167-
std::chrono::system_clock::duration>(std::chrono::system_clock::now()
168-
+ duration);
167+
auto to_wait = chrono_utils::ceil<
168+
std::chrono::system_clock::duration>(duration);
169+
auto deadline = std::chrono::system_clock::now() + to_wait;
169170
return cond_wait(handle, deadline);
170171
}
171172
inline bool cond_wait(cond_handle &handle,
172173
std::chrono::system_clock::time_point deadline) {
173-
auto ns = std::chrono::duration_cast<std::chrono::nanoseconds>(
174+
auto ns = chrono_utils::ceil<std::chrono::nanoseconds>(
174175
deadline.time_since_epoch()).count();
175176
struct ::timespec ts = { ::time_t(ns / 1000000000), long(ns % 1000000000) };
176177
SWIFT_PTHREADS_RETURN_TRUE_OR_FALSE(

‎include/swift/Threading/Impl/Win32.h

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,8 @@
1919

2020
#include "Win32/Win32Defs.h"
2121

22-
#include <chrono>
22+
#include "chrono.h"
23+
2324
#include <atomic>
2425

2526
#include "llvm/ADT/Optional.h"
@@ -120,7 +121,7 @@ inline void cond_wait(cond_handle &handle) {
120121
template <class Rep, class Period>
121122
inline bool cond_wait(cond_handle &handle,
122123
std::chrono::duration<Rep, Period> duration) {
123-
auto ms = std::chrono::duration_cast<std::chrono::milliseconds>(duration);
124+
auto ms = chrono_utils::ceil<std::chrono::milliseconds>(duration);
124125
return SleepConditionVariableSRW(&handle.condition,
125126
&handle.lock,
126127
DWORD(ms.count()),

‎include/swift/Threading/Impl/chrono.h

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
//===--- chrono.h - Utility functions for duration/time_point - -*- C++ -*-===//
2+
//
3+
// This source file is part of the Swift.org open source project
4+
//
5+
// Copyright (c) 2022 Apple Inc. and the Swift project authors
6+
// Licensed under Apache License v2.0 with Runtime Library Exception
7+
//
8+
// See https://swift.org/LICENSE.txt for license information
9+
// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
10+
//
11+
//===----------------------------------------------------------------------===//
12+
//
13+
// Specifically, we want ceil() for these types, but that's only available in
14+
// C++17, and we need to build with C++14, so... include a version of the
15+
// necesary code here.
16+
//
17+
//===----------------------------------------------------------------------===//
18+
19+
#ifndef SWIFT_THREADING_IMPL_CHRONO_H
20+
#define SWIFT_THREADING_IMPL_CHRONO_H
21+
22+
#include <chrono>
23+
#include <type_traits>
24+
25+
namespace swift {
26+
namespace threading_impl {
27+
namespace chrono_utils {
28+
29+
#if __cplusplus >= 201703L
30+
using std::chrono::ceil;
31+
#else
32+
33+
namespace detail {
34+
template <class _Tp>
35+
struct is_duration : std::false_type {};
36+
37+
template <class _Rep, class _Period>
38+
struct is_duration<std::chrono::duration<_Rep, _Period> >
39+
: std::true_type {};
40+
41+
template <class _Rep, class _Period>
42+
struct is_duration<const std::chrono::duration<_Rep, _Period> >
43+
: std::true_type {};
44+
45+
template <class _Rep, class _Period>
46+
struct is_duration<volatile std::chrono::duration<_Rep, _Period> >
47+
: std::true_type {};
48+
49+
template <class _Rep, class _Period>
50+
struct is_duration<const volatile std::chrono::duration<_Rep, _Period> >
51+
: std::true_type {};
52+
}
53+
54+
template <class To, class Rep, class Period,
55+
class = std::enable_if_t<detail::is_duration<To>::value>>
56+
constexpr To
57+
ceil(const std::chrono::duration<Rep, Period>& d)
58+
{
59+
To t = std::chrono::duration_cast<To>(d);
60+
if (t < d)
61+
t = t + To{1};
62+
return t;
63+
}
64+
65+
#endif
66+
67+
} // namespace chrono_utils
68+
} // namespace threading_impl
69+
} // namespace swift
70+
71+
#endif // SWIFT_THREADING_IMPL_CHRONO_H

0 commit comments

Comments
 (0)