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

Cache the folderconfig Path() call #2432

Merged
merged 1 commit into from
Nov 6, 2015
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion lib/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand Down
7 changes: 7 additions & 0 deletions lib/config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
57 changes: 35 additions & 22 deletions lib/config/folderconfiguration.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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 {
Expand Down Expand Up @@ -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"
}
Expand All @@ -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 {
Expand Down