StackExchange / StackExchange.Redis

General purpose redis client
https://stackexchange.github.io/StackExchange.Redis/
Other
5.88k stars 1.51k forks source link

Is it possible to reduce DateTime.UtcNow invoke? #2572

Closed gruan01 closed 9 months ago

gruan01 commented 12 months ago

Performance trace show DateTime.UtcNow take too many CPU . Message.ctor assign DateTime.UtcNow to CreatedDateTime, but this property seem not so necessary.

May be , provide a ISystemClock just like MemoryCacheOptions.Clock is a good choice, so we can provide a not so precise UtcNow to it, ,

    public class SlowDateTime
    {
        private static TimeSpan interval;
        private static DateTime _now = DateTime.Now;
        public static DateTime Now => _now;
        private static DateTime _utcNow = DateTime.UtcNow;
        public static DateTime UtcNow => _utcNow;

        public static void Init(TimeSpan interval)
        {
            SlowDateTime.interval = interval;
            Refresh();
        }

        private static void Refresh()
        {
            Task.Factory.StartNew(async () =>
            {
                while (true)
                {
                    await Task.Delay(interval);
                    _now = DateTime.Now;
                    _utcNow = DateTime.UtcNow;
                }
            }, TaskCreationOptions.LongRunning);
        }
    }

    public class SlowClock : ISystemClock
    {
        public DateTimeOffset UtcNow => SlowDateTime.UtcNow;

        public static SlowClock Default { get; }

        static SlowClock()
        {
            Default = new SlowClock();
        }
    }

Performance trace shotcuts: 1 2 3

NickCraver commented 12 months ago

We need accurate timings here for very precise timeouts (some users have ~50ms thresholds), so this isn't something we can readily change without breaking those scenarios. It's a tradeoff of cost vs. prevision overall and the issues are:

  1. Fidelity reduction even further (re purge on heartbeat now so there's an existing tolerance violation range).
  2. Relying on the threadpool to update the base time when the primary consumer in the default case is a timeout algorithm means we're relying on the exhausted resource to clear the way for purging said resource creating spikes in spiral-of-death scenarios.

In short: I get what you're talking about, and have done similar elsewhere, but it's not a good use case here.