Merge pull request #1320 from AudriusButkevicius/cache

Remove fd cache (ref #1308)

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"

Godeps/_workspace/src/github.com/AudriusButkevicius/lfu-go/LICENSE

@@ -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.

Godeps/_workspace/src/github.com/AudriusButkevicius/lfu-go/README.md

@@ -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)
-```

Godeps/_workspace/src/github.com/AudriusButkevicius/lfu-go/lfu.go

@@ -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)
-	}
-}

Godeps/_workspace/src/github.com/AudriusButkevicius/lfu-go/lfu_test.go

@@ -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")
-	}
-}

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
 		}

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
 	}
 }