From 380d5dfa6d4242481b32317f0c9b8b2280ae715b Mon Sep 17 00:00:00 2001 From: Audrius Butkevicius Date: Wed, 28 Jan 2015 14:32:59 +0000 Subject: [PATCH] Remove fd cache (ref #1308) --- Godeps/Godeps.json | 4 - .../AudriusButkevicius/lfu-go/LICENSE | 19 --- .../AudriusButkevicius/lfu-go/README.md | 19 --- .../AudriusButkevicius/lfu-go/lfu.go | 156 ------------------ .../AudriusButkevicius/lfu-go/lfu_test.go | 68 -------- internal/model/model.go | 4 +- internal/model/puller.go | 34 +--- 7 files changed, 7 insertions(+), 297 deletions(-) delete mode 100644 Godeps/_workspace/src/github.com/AudriusButkevicius/lfu-go/LICENSE delete mode 100644 Godeps/_workspace/src/github.com/AudriusButkevicius/lfu-go/README.md delete mode 100644 Godeps/_workspace/src/github.com/AudriusButkevicius/lfu-go/lfu.go delete mode 100644 Godeps/_workspace/src/github.com/AudriusButkevicius/lfu-go/lfu_test.go diff --git a/Godeps/Godeps.json b/Godeps/Godeps.json index 840ea1ca..25ce354e 100644 --- a/Godeps/Godeps.json +++ b/Godeps/Godeps.json @@ -5,10 +5,6 @@ "./cmd/..." ], "Deps": [ - { - "ImportPath": "github.com/AudriusButkevicius/lfu-go", - "Rev": "164bcecceb92fd6037f4d18a8d97b495ec6ef669" - }, { "ImportPath": "github.com/bkaradzic/go-lz4", "Rev": "93a831dcee242be64a9cc9803dda84af25932de7" diff --git a/Godeps/_workspace/src/github.com/AudriusButkevicius/lfu-go/LICENSE b/Godeps/_workspace/src/github.com/AudriusButkevicius/lfu-go/LICENSE deleted file mode 100644 index 431a0037..00000000 --- a/Godeps/_workspace/src/github.com/AudriusButkevicius/lfu-go/LICENSE +++ /dev/null @@ -1,19 +0,0 @@ -Copyright (C) 2012 Dave Grijalva - -Permission is hereby granted, free of charge, to any person obtaining a -copy of this software and associated documentation files (the "Software"), -to deal in the Software without restriction, including without limitation -the rights to use, copy, modify, merge, publish, distribute, sublicense, -and/or sell copies of the Software, and to permit persons to whom the -Software is furnished to do so, subject to the following conditions: - -The above copyright notice and this permission notice shall be included -in all copies or substantial portions of the Software. - -THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS -OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, -FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL -THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER -LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, -OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN -THE SOFTWARE. \ No newline at end of file diff --git a/Godeps/_workspace/src/github.com/AudriusButkevicius/lfu-go/README.md b/Godeps/_workspace/src/github.com/AudriusButkevicius/lfu-go/README.md deleted file mode 100644 index 2a074258..00000000 --- a/Godeps/_workspace/src/github.com/AudriusButkevicius/lfu-go/README.md +++ /dev/null @@ -1,19 +0,0 @@ -A simple LFU cache for golang. Based on the paper [An O(1) algorithm for implementing the LFU cache eviction scheme](http://dhruvbird.com/lfu.pdf). - -Usage: - -```go -import "github.com/dgrijalva/lfu-go" - -// Make a new thing -c := lfu.New() - -// Set some values -c.Set("myKey", myValue) - -// Retrieve some values -myValue = c.Get("myKey") - -// Evict some values -c.Evict(1) -``` \ No newline at end of file diff --git a/Godeps/_workspace/src/github.com/AudriusButkevicius/lfu-go/lfu.go b/Godeps/_workspace/src/github.com/AudriusButkevicius/lfu-go/lfu.go deleted file mode 100644 index cfe38715..00000000 --- a/Godeps/_workspace/src/github.com/AudriusButkevicius/lfu-go/lfu.go +++ /dev/null @@ -1,156 +0,0 @@ -package lfu - -import ( - "container/list" - "sync" -) - -type Eviction struct { - Key string - Value interface{} -} - -type Cache struct { - // If len > UpperBound, cache will automatically evict - // down to LowerBound. If either value is 0, this behavior - // is disabled. - UpperBound int - LowerBound int - values map[string]*cacheEntry - freqs *list.List - len int - lock *sync.Mutex - EvictionChannel chan<- Eviction -} - -type cacheEntry struct { - key string - value interface{} - freqNode *list.Element -} - -type listEntry struct { - entries map[*cacheEntry]byte - freq int -} - -func New() *Cache { - c := new(Cache) - c.values = make(map[string]*cacheEntry) - c.freqs = list.New() - c.lock = new(sync.Mutex) - return c -} - -func (c *Cache) Get(key string) interface{} { - c.lock.Lock() - defer c.lock.Unlock() - if e, ok := c.values[key]; ok { - c.increment(e) - return e.value - } - return nil -} - -func (c *Cache) Set(key string, value interface{}) { - c.lock.Lock() - defer c.lock.Unlock() - if e, ok := c.values[key]; ok { - // value already exists for key. overwrite - e.value = value - c.increment(e) - } else { - // value doesn't exist. insert - e := new(cacheEntry) - e.key = key - e.value = value - c.values[key] = e - c.increment(e) - c.len++ - // bounds mgmt - if c.UpperBound > 0 && c.LowerBound > 0 { - if c.len > c.UpperBound { - c.evict(c.len - c.LowerBound) - } - } - } -} - -func (c *Cache) Len() int { - c.lock.Lock() - defer c.lock.Unlock() - return c.len -} - -func (c *Cache) Evict(count int) int { - c.lock.Lock() - defer c.lock.Unlock() - return c.evict(count) -} - -func (c *Cache) evict(count int) int { - // No lock here so it can be called - // from within the lock (during Set) - var evicted int - for i := 0; i < count; { - if place := c.freqs.Front(); place != nil { - for entry, _ := range place.Value.(*listEntry).entries { - if i < count { - if c.EvictionChannel != nil { - c.EvictionChannel <- Eviction{ - Key: entry.key, - Value: entry.value, - } - } - delete(c.values, entry.key) - c.remEntry(place, entry) - evicted++ - c.len-- - i++ - } - } - } - } - return evicted -} - -func (c *Cache) increment(e *cacheEntry) { - currentPlace := e.freqNode - var nextFreq int - var nextPlace *list.Element - if currentPlace == nil { - // new entry - nextFreq = 1 - nextPlace = c.freqs.Front() - } else { - // move up - nextFreq = currentPlace.Value.(*listEntry).freq + 1 - nextPlace = currentPlace.Next() - } - - if nextPlace == nil || nextPlace.Value.(*listEntry).freq != nextFreq { - // create a new list entry - li := new(listEntry) - li.freq = nextFreq - li.entries = make(map[*cacheEntry]byte) - if currentPlace != nil { - nextPlace = c.freqs.InsertAfter(li, currentPlace) - } else { - nextPlace = c.freqs.PushFront(li) - } - } - e.freqNode = nextPlace - nextPlace.Value.(*listEntry).entries[e] = 1 - if currentPlace != nil { - // remove from current position - c.remEntry(currentPlace, e) - } -} - -func (c *Cache) remEntry(place *list.Element, entry *cacheEntry) { - entries := place.Value.(*listEntry).entries - delete(entries, entry) - if len(entries) == 0 { - c.freqs.Remove(place) - } -} diff --git a/Godeps/_workspace/src/github.com/AudriusButkevicius/lfu-go/lfu_test.go b/Godeps/_workspace/src/github.com/AudriusButkevicius/lfu-go/lfu_test.go deleted file mode 100644 index 61d97c0a..00000000 --- a/Godeps/_workspace/src/github.com/AudriusButkevicius/lfu-go/lfu_test.go +++ /dev/null @@ -1,68 +0,0 @@ -package lfu - -import ( - "fmt" - "testing" -) - -func TestLFU(t *testing.T) { - c := New() - c.Set("a", "a") - if v := c.Get("a"); v != "a" { - t.Errorf("Value was not saved: %v != 'a'", v) - } - if l := c.Len(); l != 1 { - t.Errorf("Length was not updated: %v != 1", l) - } - - c.Set("b", "b") - if v := c.Get("b"); v != "b" { - t.Errorf("Value was not saved: %v != 'b'", v) - } - if l := c.Len(); l != 2 { - t.Errorf("Length was not updated: %v != 2", l) - } - - c.Get("a") - evicted := c.Evict(1) - if v := c.Get("a"); v != "a" { - t.Errorf("Value was improperly evicted: %v != 'a'", v) - } - if v := c.Get("b"); v != nil { - t.Errorf("Value was not evicted: %v", v) - } - if l := c.Len(); l != 1 { - t.Errorf("Length was not updated: %v != 1", l) - } - if evicted != 1 { - t.Errorf("Number of evicted items is wrong: %v != 1", evicted) - } -} - -func TestBoundsMgmt(t *testing.T) { - c := New() - c.UpperBound = 10 - c.LowerBound = 5 - - for i := 0; i < 100; i++ { - c.Set(fmt.Sprintf("%v", i), i) - } - if c.Len() > 10 { - t.Errorf("Bounds management failed to evict properly: %v", c.Len()) - } -} - -func TestEviction(t *testing.T) { - ch := make(chan Eviction, 1) - - c := New() - c.EvictionChannel = ch - c.Set("a", "b") - c.Evict(1) - - ev := <-ch - - if ev.Key != "a" || ev.Value.(string) != "b" { - t.Error("Incorrect item") - } -} diff --git a/internal/model/model.go b/internal/model/model.go index a505e3a9..daddd1d0 100644 --- a/internal/model/model.go +++ b/internal/model/model.go @@ -755,7 +755,9 @@ func (m *Model) Request(deviceID protocol.DeviceID, folder, name string, offset } reader = strings.NewReader(target) } else { - reader, err = os.Open(fn) // XXX: Inefficient, should cache fd? + // Cannot easily cache fd's because we might need to delete the file + // at any moment. + reader, err = os.Open(fn) if err != nil { return nil, err } diff --git a/internal/model/puller.go b/internal/model/puller.go index c0fbccbb..da0b3e14 100644 --- a/internal/model/puller.go +++ b/internal/model/puller.go @@ -25,8 +25,6 @@ import ( "sync" "time" - "github.com/AudriusButkevicius/lfu-go" - "github.com/syncthing/protocol" "github.com/syncthing/syncthing/internal/config" "github.com/syncthing/syncthing/internal/db" @@ -698,19 +696,6 @@ func (p *Puller) copierRoutine(in <-chan copyBlocksState, pullChan chan<- pullBl continue } - evictionChan := make(chan lfu.Eviction) - - fdCache := lfu.New() - fdCache.UpperBound = 50 - fdCache.LowerBound = 20 - fdCache.EvictionChannel = evictionChan - - go func() { - for item := range evictionChan { - item.Value.(*os.File).Close() - } - }() - folderRoots := make(map[string]string) p.model.fmut.RLock() for folder, cfg := range p.model.folderCfgs { @@ -721,22 +706,13 @@ func (p *Puller) copierRoutine(in <-chan copyBlocksState, pullChan chan<- pullBl for _, block := range state.blocks { buf = buf[:int(block.Size)] found := p.model.finder.Iterate(block.Hash, func(folder, file string, index int32) bool { - path := filepath.Join(folderRoots[folder], file) - - var fd *os.File - - fdi := fdCache.Get(path) - if fdi != nil { - fd = fdi.(*os.File) - } else { - fd, err = os.Open(path) - if err != nil { - return false - } - fdCache.Set(path, fd) + fd, err := os.Open(filepath.Join(folderRoots[folder], file)) + if err != nil { + return false } _, err = fd.ReadAt(buf, protocol.BlockSize*int64(index)) + fd.Close() if err != nil { return false } @@ -782,8 +758,6 @@ func (p *Puller) copierRoutine(in <-chan copyBlocksState, pullChan chan<- pullBl state.copyDone() } } - fdCache.Evict(fdCache.Len()) - close(evictionChan) out <- state.sharedPullerState } }