Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace time.Now with lower precision time source #51

Open
bartle-stripe opened this issue Dec 13, 2018 · 9 comments
Open

Replace time.Now with lower precision time source #51

bartle-stripe opened this issue Dec 13, 2018 · 9 comments

Comments

@bartle-stripe
Copy link

time.Now() will sometimes result in a full system call (this is the default on AWS EC2 instances). I think for an LRU you can probably get away with second precision. One option would be to have a background goroutine that called time.Now() every second and cached the result globally.

@coocood
Copy link
Owner

coocood commented Dec 13, 2018

Do you know any cache library have this optimization?

@pheepi
Copy link
Contributor

pheepi commented Jan 14, 2020

I have proposed to replace wall time time.Now() by a custom timer in #69. Then you could create you own custom time structure with field seconds and update it periodically with a Go timer.

@bartle-stripe
Copy link
Author

Seems reasonable to me.

@thesilentg
Copy link

@pheepi @bartle-stripe Did you implement the background goroutine time.Now() timer? If so, would you be willing to share the code here?

@coocood For reference, this is what the pprof data looks like when trying to set tens of millions of entries in an AWS Fargate task. Roughly 60% of the CPU for setting items is being spent in time.now()

image

@bartle-stripe
Copy link
Author

I don't believe I ever did.

@thesilentg
Copy link

thesilentg commented Jul 24, 2020

This resulted in ~50% speedup for calls to .Set()

type lowResClock struct {
	now uint32
}

func newLosResClock() lowResClock {
	clock := lowResClock{
		now: uint32(time.Now().Unix()),
	}
	ticker := time.NewTicker(1 * time.Second)
	go func() {
		for {
			select {
			case _ = <-ticker.C:
				clock.now = uint32(time.Now().Unix())
			}
		}
	}()
	return clock
}

func (clock lowResClock) Now() uint32 {
	return clock.now
}

@thesilentg
Copy link

@coocood Do you think the performance improvement from this is worth replacing the existing default implementation?

@coocood
Copy link
Owner

coocood commented Jul 26, 2020

@thesilentg

The lowResClock starts a goroutine and never exit, if cache are created and discarded too many times, there would be a lot of goroutines leaked.

And clock.now needs atomic access to be race-free.

We could create a well-implemented builtin lowResClock, but I think the default should still be time.Now()

@pheepi
Copy link
Contributor

pheepi commented Aug 12, 2020

Optimized timer with cached value is now part of the code passed as a custom parameter of NewCacheCustomTimer in #88. Create new stoppable timer with NewCachedTimer and call Stop() method when your cache is no more in use. The method stops the background routine that periodically updates the time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants