|
| 1 | +2016-11-04 Filip Pizlo < [email protected]> |
| 2 | + |
| 3 | + WTF::ParkingLot should stop using std::chrono because std::chrono::duration casts are prone to overflows |
| 4 | + https://bugs.webkit.org/show_bug.cgi?id=152045 |
| 5 | + |
| 6 | + Reviewed by Andy Estes. |
| 7 | + |
| 8 | + We used to use 'double' for all time measurements. Sometimes it was milliseconds, |
| 9 | + sometimes it was seconds. Sometimes we measured a span of time, sometimes we spoke of time |
| 10 | + since some epoch. When we spoke of time since epoch, we either used a monotonic clock or |
| 11 | + a wall clock. The type - always 'double' - never told us what kind of time we had, even |
| 12 | + though there were roughly six of them (sec interval, ms interval, sec since epoch on wall, |
| 13 | + ms since epoch on wall, sec since epoch monotonic, ms since epoch monotonic). |
| 14 | + |
| 15 | + At some point, we thought that it would be a good idea to replace these doubles with |
| 16 | + std::chrono. But since replacing some things with std::chrono, we found it to be terribly |
| 17 | + inconvenient: |
| 18 | + |
| 19 | + - Outrageous API. I never want to say std::chrono::milliseconds(blah). I never want to say |
| 20 | + std::chrono::steady_clock::timepoint. The syntax for duration_cast is ugly, and ideally |
| 21 | + duration_cast would not even be a thing. |
| 22 | + |
| 23 | + - No overflow protection. std::chrono uses integers by default and using anything else is |
| 24 | + clumsy. But the integer math is done without regard for the rough edges of integer math, |
| 25 | + so any cast between std::chrono types risks overflow. Any comparison risks overflow |
| 26 | + because it may do conversions silently. We have even found bugs where some C++ |
| 27 | + implementations had more overflows than others, which ends up being a special kind of |
| 28 | + hell. In many cases, the overflow also has nasal demons. |
| 29 | + |
| 30 | + It's an error to represent time using integers. It would have been excusable back when |
| 31 | + floating point math was not guaranteed to be supported on all platforms, but that would |
| 32 | + have been a long time ago. Time is a continuous, infinite concept and it's a perfect fit |
| 33 | + for floating point: |
| 34 | + |
| 35 | + - Floating point preserves precision under multiplication in all but extreme cases, so |
| 36 | + using floating point for time means that unit conversions are almost completely |
| 37 | + lossless. This means that we don't have to think very hard about what units to use. In |
| 38 | + this patch, we use seconds almost everywhere. We only convert at boundaries, like an API |
| 39 | + boundary that wants something other than seconds. |
| 40 | + |
| 41 | + - Floating point makes it easy to reason about infinity, which is something that time code |
| 42 | + wants to do a lot. Example: when would you like to timeout? Infinity please! This is the |
| 43 | + most elegant way of having an API support both a timeout variant and a no-timeout |
| 44 | + variant. |
| 45 | + |
| 46 | + - Floating point does well-understood things when math goes wrong, and these things are |
| 47 | + pretty well optimized to match what a mathematician would do when computing with real |
| 48 | + numbers represented using scientific notation with a finite number of significant |
| 49 | + digits. This means that time math under floating point looks like normal math. On the |
| 50 | + other hand, std::chrono time math looks like garbage because you have to always check |
| 51 | + for multiple possible UB corners whenever you touch large integers. Integers that |
| 52 | + represent time are very likely to be large and you don't have to do much to overflow |
| 53 | + them. At this time, based on the number of bugs we have already seen due to chrono |
| 54 | + overflows, I am not certain that we even understand what are all of the corner cases |
| 55 | + that we should even check for. |
| 56 | + |
| 57 | + This patch introduces a new set of timekeeping classes that are all based on double, and |
| 58 | + all internally use seconds. These classes support algebraic typing. The classes are: |
| 59 | + |
| 60 | + - Seconds: this is for measuring a duration. |
| 61 | + - WallTime: time since epoch according to a wall clock (aka real time clock). |
| 62 | + - MonotonicTime: time since epoch according to a monotonic clock. |
| 63 | + - ClockType: enum that says either Wall or Monotonic. |
| 64 | + - TimeWithDynamicClockType: a tuple of double and ClockType, which represents either a |
| 65 | + wall time or a monotonic time. |
| 66 | + |
| 67 | + All of these classes behave like C++ values and are cheap to copy around since they are |
| 68 | + very nearly POD. This supports comprehensive conversions between the various time types. |
| 69 | + Most of this is by way of algebra. Here are just some of the rules we recognize: |
| 70 | + |
| 71 | + WallTime = WallTime + Seconds |
| 72 | + Seconds = WallTime - WallTime |
| 73 | + MonotonicTime = MonotonicTime + Seconds |
| 74 | + etc... |
| 75 | + |
| 76 | + We support negative, infinite, and NaN times because math. |
| 77 | + |
| 78 | + We support conversions between MonotonicTime and WallTime, like: |
| 79 | + |
| 80 | + WallTime wt = mt.approximateWallTime() |
| 81 | + |
| 82 | + This is called this "approximate" because the only way to do it is to get the current time |
| 83 | + on both clocks and convert relative to that. |
| 84 | + |
| 85 | + Many of our APIs would be happy using whatever notion of time the user wanted to use. For |
| 86 | + those APIs, which includes Condition and ParkingLot, we have TimeWithDynamicClockType. You |
| 87 | + can automatically convert WallTime or MonotonicTime to TimeWithDynamicClockType. This |
| 88 | + means that if you use a WallTime with Condition::waitUntil, then Condition's internal |
| 89 | + logic for when it should wake up makes its decision based on the current WallTime - but if |
| 90 | + you use MonotonicTime then waitUntil will make its decision based on current |
| 91 | + MonotonicTime. This is a greater level of flexibility than chrono allowed, since chrono |
| 92 | + did not have the concept of a dynamic clock type. |
| 93 | + |
| 94 | + This patch does not include conversions between std::chrono and these new time classes, |
| 95 | + because past experience shows that we're quite bad at getting conversions between |
| 96 | + std::chrono and anything else right. Also, I didn't need such conversion code because this |
| 97 | + patch only converts code that transitively touches ParkingLot and Condition. It was easy |
| 98 | + to get all of that code onto the new time classes. |
| 99 | + |
| 100 | + * WTF.xcodeproj/project.pbxproj: |
| 101 | + * wtf/AutomaticThread.cpp: |
| 102 | + (WTF::AutomaticThread::start): |
| 103 | + * wtf/CMakeLists.txt: |
| 104 | + * wtf/ClockType.cpp: Added. |
| 105 | + (WTF::printInternal): |
| 106 | + * wtf/ClockType.h: Added. |
| 107 | + * wtf/Condition.h: |
| 108 | + (WTF::ConditionBase::waitUntil): |
| 109 | + (WTF::ConditionBase::waitFor): |
| 110 | + (WTF::ConditionBase::wait): |
| 111 | + (WTF::ConditionBase::waitUntilWallClockSeconds): Deleted. |
| 112 | + (WTF::ConditionBase::waitUntilMonotonicClockSeconds): Deleted. |
| 113 | + (WTF::ConditionBase::waitForSeconds): Deleted. |
| 114 | + (WTF::ConditionBase::waitForSecondsImpl): Deleted. |
| 115 | + (WTF::ConditionBase::waitForImpl): Deleted. |
| 116 | + (WTF::ConditionBase::absoluteFromRelative): Deleted. |
| 117 | + * wtf/CrossThreadQueue.h: |
| 118 | + (WTF::CrossThreadQueue<DataType>::waitForMessage): |
| 119 | + * wtf/CurrentTime.cpp: |
| 120 | + (WTF::sleep): |
| 121 | + * wtf/MessageQueue.h: |
| 122 | + (WTF::MessageQueue::infiniteTime): Deleted. |
| 123 | + * wtf/MonotonicTime.cpp: Added. |
| 124 | + (WTF::MonotonicTime::now): |
| 125 | + (WTF::MonotonicTime::approximateWallTime): |
| 126 | + (WTF::MonotonicTime::dump): |
| 127 | + (WTF::MonotonicTime::sleep): |
| 128 | + * wtf/MonotonicTime.h: Added. |
| 129 | + (WTF::MonotonicTime::MonotonicTime): |
| 130 | + (WTF::MonotonicTime::fromRawDouble): |
| 131 | + (WTF::MonotonicTime::infinity): |
| 132 | + (WTF::MonotonicTime::secondsSinceEpoch): |
| 133 | + (WTF::MonotonicTime::approximateMonotonicTime): |
| 134 | + (WTF::MonotonicTime::operator bool): |
| 135 | + (WTF::MonotonicTime::operator+): |
| 136 | + (WTF::MonotonicTime::operator-): |
| 137 | + (WTF::MonotonicTime::operator+=): |
| 138 | + (WTF::MonotonicTime::operator-=): |
| 139 | + (WTF::MonotonicTime::operator==): |
| 140 | + (WTF::MonotonicTime::operator!=): |
| 141 | + (WTF::MonotonicTime::operator<): |
| 142 | + (WTF::MonotonicTime::operator>): |
| 143 | + (WTF::MonotonicTime::operator<=): |
| 144 | + (WTF::MonotonicTime::operator>=): |
| 145 | + * wtf/ParkingLot.cpp: |
| 146 | + (WTF::ParkingLot::parkConditionallyImpl): |
| 147 | + (WTF::ParkingLot::unparkOne): |
| 148 | + (WTF::ParkingLot::unparkOneImpl): |
| 149 | + (WTF::ParkingLot::unparkCount): |
| 150 | + * wtf/ParkingLot.h: |
| 151 | + (WTF::ParkingLot::parkConditionally): |
| 152 | + (WTF::ParkingLot::compareAndPark): |
| 153 | + * wtf/Seconds.cpp: Added. |
| 154 | + (WTF::Seconds::operator+): |
| 155 | + (WTF::Seconds::operator-): |
| 156 | + (WTF::Seconds::dump): |
| 157 | + (WTF::Seconds::sleep): |
| 158 | + * wtf/Seconds.h: Added. |
| 159 | + (WTF::Seconds::Seconds): |
| 160 | + (WTF::Seconds::value): |
| 161 | + (WTF::Seconds::seconds): |
| 162 | + (WTF::Seconds::milliseconds): |
| 163 | + (WTF::Seconds::microseconds): |
| 164 | + (WTF::Seconds::nanoseconds): |
| 165 | + (WTF::Seconds::fromMilliseconds): |
| 166 | + (WTF::Seconds::fromMicroseconds): |
| 167 | + (WTF::Seconds::fromNanoseconds): |
| 168 | + (WTF::Seconds::infinity): |
| 169 | + (WTF::Seconds::operator bool): |
| 170 | + (WTF::Seconds::operator+): |
| 171 | + (WTF::Seconds::operator-): |
| 172 | + (WTF::Seconds::operator*): |
| 173 | + (WTF::Seconds::operator/): |
| 174 | + (WTF::Seconds::operator+=): |
| 175 | + (WTF::Seconds::operator-=): |
| 176 | + (WTF::Seconds::operator*=): |
| 177 | + (WTF::Seconds::operator/=): |
| 178 | + (WTF::Seconds::operator==): |
| 179 | + (WTF::Seconds::operator!=): |
| 180 | + (WTF::Seconds::operator<): |
| 181 | + (WTF::Seconds::operator>): |
| 182 | + (WTF::Seconds::operator<=): |
| 183 | + (WTF::Seconds::operator>=): |
| 184 | + * wtf/TimeWithDynamicClockType.cpp: Added. |
| 185 | + (WTF::TimeWithDynamicClockType::now): |
| 186 | + (WTF::TimeWithDynamicClockType::nowWithSameClock): |
| 187 | + (WTF::TimeWithDynamicClockType::wallTime): |
| 188 | + (WTF::TimeWithDynamicClockType::monotonicTime): |
| 189 | + (WTF::TimeWithDynamicClockType::approximateWallTime): |
| 190 | + (WTF::TimeWithDynamicClockType::approximateMonotonicTime): |
| 191 | + (WTF::TimeWithDynamicClockType::operator-): |
| 192 | + (WTF::TimeWithDynamicClockType::operator<): |
| 193 | + (WTF::TimeWithDynamicClockType::operator>): |
| 194 | + (WTF::TimeWithDynamicClockType::operator<=): |
| 195 | + (WTF::TimeWithDynamicClockType::operator>=): |
| 196 | + (WTF::TimeWithDynamicClockType::dump): |
| 197 | + (WTF::TimeWithDynamicClockType::sleep): |
| 198 | + * wtf/TimeWithDynamicClockType.h: Added. |
| 199 | + (WTF::TimeWithDynamicClockType::TimeWithDynamicClockType): |
| 200 | + (WTF::TimeWithDynamicClockType::fromRawDouble): |
| 201 | + (WTF::TimeWithDynamicClockType::secondsSinceEpoch): |
| 202 | + (WTF::TimeWithDynamicClockType::clockType): |
| 203 | + (WTF::TimeWithDynamicClockType::withSameClockAndRawDouble): |
| 204 | + (WTF::TimeWithDynamicClockType::operator bool): |
| 205 | + (WTF::TimeWithDynamicClockType::operator+): |
| 206 | + (WTF::TimeWithDynamicClockType::operator-): |
| 207 | + (WTF::TimeWithDynamicClockType::operator+=): |
| 208 | + (WTF::TimeWithDynamicClockType::operator-=): |
| 209 | + (WTF::TimeWithDynamicClockType::operator==): |
| 210 | + (WTF::TimeWithDynamicClockType::operator!=): |
| 211 | + * wtf/WallTime.cpp: Added. |
| 212 | + (WTF::WallTime::now): |
| 213 | + (WTF::WallTime::approximateMonotonicTime): |
| 214 | + (WTF::WallTime::dump): |
| 215 | + (WTF::WallTime::sleep): |
| 216 | + * wtf/WallTime.h: Added. |
| 217 | + (WTF::WallTime::WallTime): |
| 218 | + (WTF::WallTime::fromRawDouble): |
| 219 | + (WTF::WallTime::infinity): |
| 220 | + (WTF::WallTime::secondsSinceEpoch): |
| 221 | + (WTF::WallTime::approximateWallTime): |
| 222 | + (WTF::WallTime::operator bool): |
| 223 | + (WTF::WallTime::operator+): |
| 224 | + (WTF::WallTime::operator-): |
| 225 | + (WTF::WallTime::operator+=): |
| 226 | + (WTF::WallTime::operator-=): |
| 227 | + (WTF::WallTime::operator==): |
| 228 | + (WTF::WallTime::operator!=): |
| 229 | + (WTF::WallTime::operator<): |
| 230 | + (WTF::WallTime::operator>): |
| 231 | + (WTF::WallTime::operator<=): |
| 232 | + (WTF::WallTime::operator>=): |
| 233 | + * wtf/threads/BinarySemaphore.cpp: |
| 234 | + (WTF::BinarySemaphore::wait): |
| 235 | + * wtf/threads/BinarySemaphore.h: |
| 236 | + |
1 | 237 | 2016-11-03 Filip Pizlo < [email protected]>
|
2 | 238 |
|
3 | 239 | DFG plays fast and loose with the shadow values of a Phi
|
|
0 commit comments