-
Notifications
You must be signed in to change notification settings - Fork 3
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
Cache static files in memory #15
base: main
Are you sure you want to change the base?
Cache static files in memory #15
Conversation
writer := httputil.NewTimeoutResponseWriter(w, 10*time.Second) | ||
http.ServeContent(writer, r, path.Base(r.URL.Path), info.ModTime, cacheReader) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This logic is duplicated, please refactor.
internal/cache/contentcache.go
Outdated
return &ContentCache{m: m, cache: cache}, nil | ||
} | ||
|
||
func (c *ContentCache) GetContent(id string, r io.Reader) (*bytes.Buffer, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This signature is inappropiate, caches should return what is put in.
internal/cache/contentcache.go
Outdated
func NewContentCache() (*ContentCache, error) { | ||
m := make(map[string]*sync.Mutex) | ||
cache, err := ristretto.NewCache(&ristretto.Config{ | ||
NumCounters: 1e7, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How is this number derived?
internal/cache/contentcache.go
Outdated
m := make(map[string]*sync.Mutex) | ||
cache, err := ristretto.NewCache(&ristretto.Config{ | ||
NumCounters: 1e7, | ||
MaxCost: contentCacheSize, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be passed in as argument.
internal/cache/contentcache.go
Outdated
BufferItems: 64, | ||
OnExit: func(item interface{}) { | ||
cell := item.(contentCacheCell) | ||
delete(m, cell.hash) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Access to this map should be synchronized.
Please write a test for the ContentCache:
|
Instead of storing key and value as value, use a reference-counted map of rwmutexes (it automatically deletes mutexes so no need onExit in Ristretto) Separated into getContent and setContent in order to allow getContent call without io.ReadSeeker Used generics
b74cf6c
to
9c1086c
Compare
No description provided.