| commit | 9d64e1ef74ac803bb7b8708c814f5b3baed6f8ff | |
| author | Stephen Toub <stoub@microsoft.com> | |
| Tue, 9 Jul 2019 13:26:38 +0000 (9 09:26 -0400) | ||
| committer | Marek Safar <marek.safar@gmail.com> | |
| Wed, 24 Jul 2019 07:45:03 +0000 (24 09:45 +0200) | ||
| tree | b04a7ce9e5cfa93b3c8ae5f8ea5da47ff4994500 | treesnapshot (tar.gz zip) |
| parent | 70c72891f2695614d9609f47a42161c85f83e836 | commitdiff |
Fix regression in Timers with long timeouts (dotnet/coreclr#25604)
* Fix regression in Timers with long timeouts
Early in .NET Core 3.0, we added an important optimization to significantly reduce lock contention and CPU overheads in situations where lots of timers were firing. The optimization worked by splitting the set of timers into those expected to fire in the very near future and then everything else. However, for really long timers (e.g. > 24 days), due to integer overflows we can accidentally put a long timer onto the short list and cause a timer that shouldn’t fire for days to instead fire in milliseconds. This is not the first such bug we’ve had in Timer, and there is a fair amount of complicated casting (that we sometimes get wrong, obviously) in order to keep the tick count usage within the Int32 domain. This PR addresses the problem (and hopefully others in the future) by switching over to using Int64. We’re already paying to get 64-bit tick counts on both Windows and Unix, so this just removes truncation that was being performed, does a little more addition/comparison with 64-bit values instead of with 32-bit, and stores an Int64 instead of Int32 in each TimerQueueTimer. On 64-bit, this has a 0 increase in memory consumption, as it simply ends up utilizing padding space that was previously in TimerQueueTimer. On 32-bit, it increases memory allocation when creating a Timer by 4 bytes from 80 to 84 (still 3 allocations), but I believe it’s the right trade-off for reduced complexity and bug likelihood.
* Address PR feedback
Signed-off-by: dotnet-bot <dotnet-bot@microsoft.com>
* Fix regression in Timers with long timeouts
Early in .NET Core 3.0, we added an important optimization to significantly reduce lock contention and CPU overheads in situations where lots of timers were firing. The optimization worked by splitting the set of timers into those expected to fire in the very near future and then everything else. However, for really long timers (e.g. > 24 days), due to integer overflows we can accidentally put a long timer onto the short list and cause a timer that shouldn’t fire for days to instead fire in milliseconds. This is not the first such bug we’ve had in Timer, and there is a fair amount of complicated casting (that we sometimes get wrong, obviously) in order to keep the tick count usage within the Int32 domain. This PR addresses the problem (and hopefully others in the future) by switching over to using Int64. We’re already paying to get 64-bit tick counts on both Windows and Unix, so this just removes truncation that was being performed, does a little more addition/comparison with 64-bit values instead of with 32-bit, and stores an Int64 instead of Int32 in each TimerQueueTimer. On 64-bit, this has a 0 increase in memory consumption, as it simply ends up utilizing padding space that was previously in TimerQueueTimer. On 32-bit, it increases memory allocation when creating a Timer by 4 bytes from 80 to 84 (still 3 allocations), but I believe it’s the right trade-off for reduced complexity and bug likelihood.
* Address PR feedback
Signed-off-by: dotnet-bot <dotnet-bot@microsoft.com>