From 3c87984511180d7bf865574123195588ea8c044c Mon Sep 17 00:00:00 2001 From: Sergey Matveev Date: Wed, 23 Feb 2022 14:53:53 +0300 Subject: [PATCH] Do not overwrite unchanged target --- dep.go | 12 ++++---- doc/news.texi | 4 +++ run.go | 65 +++++++++++++++++++++++++++++++---------- t/goredo-empty-dep.t | 1 + t/goredo-rel-ifcreate.t | 1 + 5 files changed, 63 insertions(+), 20 deletions(-) diff --git a/dep.go b/dep.go index 7c4e35a..0e4f24f 100644 --- a/dep.go +++ b/dep.go @@ -86,7 +86,7 @@ func fileHash(fd *os.File) (string, error) { return hex.EncodeToString(h.Sum(nil)), nil } -func depWrite(fdDep *os.File, cwd, tgt string) error { +func depWrite(fdDep *os.File, cwd, tgt, hsh string) error { tracef(CDebug, "ifchange: %s <- %s", fdDep.Name(), tgt) fd, err := os.Open(path.Join(cwd, tgt)) if err != nil { @@ -104,9 +104,11 @@ func depWrite(fdDep *os.File, cwd, tgt string) error { if err != nil { return err } - hsh, err := fileHash(fd) - if err != nil { - return err + if hsh == "" { + hsh, err = fileHash(fd) + if err != nil { + return err + } } fields := []recfile.Field{ {Name: "Type", Value: DepTypeIfchange}, @@ -137,7 +139,7 @@ func depsWrite(fdDep *os.File, tgts []string) error { panic(err) } if _, errStat := os.Stat(tgt); errStat == nil { - err = depWrite(fdDep, tgtDir, tgtRel) + err = depWrite(fdDep, tgtDir, tgtRel, "") } else { tracef(CDebug, "ifchange: %s <- %s (non-existing)", fdDep.Name(), tgtRel) fields := []recfile.Field{ diff --git a/doc/news.texi b/doc/news.texi index 6cdbe07..088cc03 100644 --- a/doc/news.texi +++ b/doc/news.texi @@ -12,6 +12,10 @@ @item Inode's number is also stored as dependency information, just to prevent possible @code{ctime} collision of two files. +@item + Performance optimization: do not use target's temporary output file + at all, if its hash equals to already existing target's one. Just + touch existing target file instead. @end itemize @anchor{Release 1_22_0} diff --git a/run.go b/run.go index ba84152..3662405 100644 --- a/run.go +++ b/run.go @@ -128,24 +128,26 @@ func mkdirs(pth string) error { return os.MkdirAll(pth, os.FileMode(0777)) } -func isModified(cwd, redoDir, tgt string) (bool, *Inode, error) { +func isModified(cwd, redoDir, tgt string) (bool, *Inode, string, error) { fdDep, err := os.Open(path.Join(redoDir, tgt+DepSuffix)) if err != nil { if os.IsNotExist(err) { - return false, nil, nil + return false, nil, "", nil } - return false, nil, err + return false, nil, "", err } defer fdDep.Close() r := recfile.NewReader(fdDep) + var modified bool var ourInode *Inode + var hshPrev string for { m, err := r.NextMap() if err != nil { if errors.Is(err, io.EOF) { break } - return false, nil, err + return false, nil, "", err } if m["Type"] != DepTypeIfchange || m["Target"] != tgt { continue @@ -153,25 +155,24 @@ func isModified(cwd, redoDir, tgt string) (bool, *Inode, error) { fd, err := os.Open(path.Join(cwd, tgt)) if err != nil { if os.IsNotExist(err) { - return false, nil, nil + return false, nil, "", nil } - return false, nil, err + return false, nil, "", err } ourInode, err = inodeFromFile(fd) fd.Close() if err != nil { - return false, nil, err + return false, nil, "", err } theirInode, err := inodeFromRec(m) if err != nil { - return false, nil, err - } - if !ourInode.Equals(theirInode) { - return true, ourInode, nil + return false, nil, "", err } + hshPrev = m["Hash"] + modified = !ourInode.Equals(theirInode) break } - return false, ourInode, nil + return modified, ourInode, hshPrev, nil } func syncDir(dir string) error { @@ -269,7 +270,7 @@ func runScript(tgtOrig string, errs chan error, traced bool) error { } // Check if target is not modified externally - modified, inodePrev, err := isModified(cwd, redoDir, tgt) + modified, inodePrev, hshPrev, err := isModified(cwd, redoDir, tgt) if err != nil { lockRelease() return TgtError{tgtOrig, err} @@ -335,7 +336,7 @@ func runScript(tgtOrig string, errs chan error, traced bool) error { runErr.DoFile = doFileRelPath } - if err = depWrite(fdDep, cwdOrig, doFileRelPath); err != nil { + if err = depWrite(fdDep, cwdOrig, doFileRelPath, ""); err != nil { cleanup() return TgtError{tgtOrig, err} } @@ -703,6 +704,39 @@ func runScript(tgtOrig string, errs chan error, traced bool) error { goto Finish } } else { + var hsh string + if hshPrev != "" { + _, err = fd.Seek(0, io.SeekStart) + if err != nil { + goto Finish + } + hsh, err = fileHash(fd) + if err != nil { + goto Finish + } + if hsh == hshPrev { + tracef(CDebug, "%s has same hash, not renaming", tgtOrig) + err = os.Remove(fd.Name()) + if err != nil { + goto Finish + } + err = os.Chtimes(path.Join(cwdOrig, tgt), finished, finished) + if err != nil { + goto Finish + } + if !NoSync { + err = syncDir(cwdOrig) + if err != nil { + goto Finish + } + } + err = depWrite(fdDep, cwdOrig, tgt, hshPrev) + if err != nil { + goto Finish + } + goto RecCommit + } + } if !NoSync { err = fd.Sync() if err != nil { @@ -719,12 +753,13 @@ func runScript(tgtOrig string, errs chan error, traced bool) error { goto Finish } } - err = depWrite(fdDep, cwdOrig, tgt) + err = depWrite(fdDep, cwdOrig, tgt, hsh) if err != nil { goto Finish } } + RecCommit: // Commit .rec if !NoSync { err = fdDep.Sync() diff --git a/t/goredo-empty-dep.t b/t/goredo-empty-dep.t index a278652..63f9e30 100755 --- a/t/goredo-empty-dep.t +++ b/t/goredo-empty-dep.t @@ -12,6 +12,7 @@ echo bar EOF redo-ifchange bar stat1=`stat bar` +sleep 1 # stat does not output fractions of seconds redo-ifchange bar stat2=`stat bar` diff --git a/t/goredo-rel-ifcreate.t b/t/goredo-rel-ifcreate.t index e642f53..61d5349 100755 --- a/t/goredo-rel-ifcreate.t +++ b/t/goredo-rel-ifcreate.t @@ -14,6 +14,7 @@ redo-ifchange c/dep EOF redo-ifchange a/b/c/file stat1=`stat a/b/c/file` +sleep 1 # stat does not output fractions of seconds redo-ifchange a/b/c/file stat2=`stat a/b/c/file` -- 2.44.0