Merge pull request #2432 from calmh/cachepath

Cache the folderconfig Path() call

lib/config/config.go

@@ -175,7 +175,7 @@ func (cfg *Configuration) prepare(myID protocol.DeviceID) {
 	cfg.Options.ListenAddress = uniqueStrings(cfg.Options.ListenAddress)
 	cfg.Options.GlobalAnnServers = uniqueStrings(cfg.Options.GlobalAnnServers)
 
-	if cfg.Version < OldestHandledVersion {
+	if cfg.Version > 0 && cfg.Version < OldestHandledVersion {
 		l.Warnf("Configuration version %d is deprecated. Attempting best effort conversion, but please verify manually.", cfg.Version)
 	}
 

lib/config/config_test.go

@@ -104,6 +104,13 @@ func TestDeviceConfig(t *testing.T) {
 			},
 		}
 
+		// The cachedPath will have been resolved to an absolute path,
+		// depending on where the tests are running. Zero it out so we don't
+		// fail based on that.
+		for i := range cfg.Folders {
+			cfg.Folders[i].cachedPath = ""
+		}
+
 		if runtime.GOOS != "windows" {
 			expectedFolders[0].RawPath += string(filepath.Separator)
 		}

lib/config/folderconfiguration.go

@@ -36,7 +36,8 @@ type FolderConfiguration struct {
 	PullerPauseS          int                         `xml:"pullerPauseS" json:"pullerPauseS"`
 	MaxConflicts          int                         `xml:"maxConflicts" json:"maxConflicts"`
 
-	Invalid string `xml:"-" json:"invalid"` // Set at runtime when there is an error, not saved
+	Invalid    string `xml:"-" json:"invalid"` // Set at runtime when there is an error, not saved
+	cachedPath string
 }
 
 type FolderDeviceConfiguration struct {
@@ -55,28 +56,11 @@ func (f FolderConfiguration) Path() string {
 	// This is intentionally not a pointer method, because things like
 	// cfg.Folders["default"].Path() should be valid.
 
-	// Attempt tilde expansion; leave unchanged in case of error
-	if path, err := osutil.ExpandTilde(f.RawPath); err == nil {
-		f.RawPath = path
-	}
-
-	// Attempt absolutification; leave unchanged in case of error
-	if !filepath.IsAbs(f.RawPath) {
-		// Abs() looks like a fairly expensive syscall on Windows, while
-		// IsAbs() is a whole bunch of string mangling. I think IsAbs() may be
-		// somewhat faster in the general case, hence the outer if...
-		if path, err := filepath.Abs(f.RawPath); err == nil {
-			f.RawPath = path
-		}
+	if f.cachedPath == "" {
+		l.Infoln("bug: uncached path call (should only happen in tests)")
+		return f.cleanedPath()
 	}
-
-	// Attempt to enable long filename support on Windows. We may still not
-	// have an absolute path here if the previous steps failed.
-	if runtime.GOOS == "windows" && filepath.IsAbs(f.RawPath) && !strings.HasPrefix(f.RawPath, `\\`) {
-		return `\\?\` + f.RawPath
-	}
-
-	return f.RawPath
+	return f.cachedPath
 }
 
 func (f *FolderConfiguration) CreateMarker() error {
@@ -129,6 +113,8 @@ func (f *FolderConfiguration) prepare() {
 		f.RawPath = f.RawPath + string(filepath.Separator)
 	}
 
+	f.cachedPath = f.cleanedPath()
+
 	if f.ID == "" {
 		f.ID = "default"
 	}
@@ -140,6 +126,33 @@ func (f *FolderConfiguration) prepare() {
 	}
 }
 
+func (f *FolderConfiguration) cleanedPath() string {
+	cleaned := f.RawPath
+
+	// Attempt tilde expansion; leave unchanged in case of error
+	if path, err := osutil.ExpandTilde(cleaned); err == nil {
+		cleaned = path
+	}
+
+	// Attempt absolutification; leave unchanged in case of error
+	if !filepath.IsAbs(cleaned) {
+		// Abs() looks like a fairly expensive syscall on Windows, while
+		// IsAbs() is a whole bunch of string mangling. I think IsAbs() may be
+		// somewhat faster in the general case, hence the outer if...
+		if path, err := filepath.Abs(cleaned); err == nil {
+			cleaned = path
+		}
+	}
+
+	// Attempt to enable long filename support on Windows. We may still not
+	// have an absolute path here if the previous steps failed.
+	if runtime.GOOS == "windows" && filepath.IsAbs(cleaned) && !strings.HasPrefix(f.RawPath, `\\`) {
+		return `\\?\` + cleaned
+	}
+
+	return cleaned
+}
+
 type FolderDeviceConfigurationList []FolderDeviceConfiguration
 
 func (l FolderDeviceConfigurationList) Less(a, b int) bool {