From 45cde1dc5fa3e4126fc7e70926efe57b69b11446 Mon Sep 17 00:00:00 2001 From: Shinichiro Hamaji Date: Mon, 25 May 2015 18:21:23 +0900 Subject: Handle cache based on their contents instead of timestamps --- eval.go | 87 ++++++++++++++++++++++++++++++++---------------------------- fileutil.go | 25 ++++++++++++++++- main.go | 18 ++++++++----- parser.go | 9 ++++++- serialize.go | 32 +++++++++++----------- 5 files changed, 104 insertions(+), 67 deletions(-) diff --git a/eval.go b/eval.go index 71d9691..52e6c13 100644 --- a/eval.go +++ b/eval.go @@ -3,28 +3,27 @@ package main import ( "bytes" "fmt" - "os" "strings" "time" ) +const ( + FILE_EXISTS = 0 + FILE_NOT_EXISTS = 1 + FILE_INCONSISTENT = 2 // Modified during kati is running. +) + type ReadMakefile struct { Filename string - // We store the current time instead of the last modified time - // of the file. If we use the last modified time, we cannot - // notice the change of this file if the modification happens - // at the same second we read the file. - // - // -1 means the file did not exist and -2 means the timestamp - // was inconsistent. - Timestamp int64 + Content []byte + State int32 } type EvalResult struct { vars Vars rules []*Rule ruleVars map[string]Vars - readMks []ReadMakefile + readMks []*ReadMakefile } type Evaluator struct { @@ -37,7 +36,7 @@ type Evaluator struct { currentScope Vars avoidIO bool hasIO bool - readMks map[string]int64 + readMks map[string]*ReadMakefile filename string lineno int @@ -48,7 +47,7 @@ func newEvaluator(vars map[string]Var) *Evaluator { outVars: make(Vars), vars: vars, outRuleVars: make(map[string]Vars), - readMks: make(map[string]int64), + readMks: make(map[string]*ReadMakefile), } } @@ -242,13 +241,13 @@ func (ev *Evaluator) LookupVarInCurrentScope(name string) Var { return ev.vars.Lookup(name) } -func (ev *Evaluator) evalIncludeFile(fname string, f *os.File) error { +func (ev *Evaluator) evalIncludeFile(fname string, c []byte) error { t := time.Now() defer func() { addStats("include", literal(fname), t) }() - Log("Reading makefile %q", f) - mk, err := ParseMakefileFd(fname, f) + Log("Reading makefile %q", fname) + mk, err := ParseMakefile(c, fname) if err != nil { return err } @@ -262,21 +261,32 @@ func (ev *Evaluator) evalIncludeFile(fname string, f *os.File) error { return nil } -func (ev *Evaluator) updateMakefileTimestamp(fn string, ts int64) { - ts2, present := ev.readMks[fn] +func (ev *Evaluator) updateReadMakefile(fn string, c []byte, st int32) { + rm, present := ev.readMks[fn] if present { - if ts2 == -1 && ts != -1 { - Warn(ev.filename, ev.lineno, "%s was created after the previous read", fn) - ev.readMks[fn] = -2 - } else if ts != -1 && ts == -1 { - Warn(ev.filename, ev.lineno, "%s was removed after the previous read", fn) - ev.readMks[fn] = -2 + switch rm.State { + case FILE_EXISTS: + if st != FILE_EXISTS { + Warn(ev.filename, ev.lineno, "%s was removed after the previous read", fn) + } else if !bytes.Equal(c, rm.Content) { + Warn(ev.filename, ev.lineno, "%s was modified after the previous read", fn) + ev.readMks[fn].State = FILE_INCONSISTENT + } + return + case FILE_NOT_EXISTS: + if st != FILE_NOT_EXISTS { + Warn(ev.filename, ev.lineno, "%s was created after the previous read", fn) + ev.readMks[fn].State = FILE_INCONSISTENT + } + case FILE_INCONSISTENT: + return } - // Just keep old value otherwise. If the content has - // been changed, we can detect the change by the - // timestamp check. } else { - ev.readMks[fn] = ts + ev.readMks[fn] = &ReadMakefile{ + Filename: fn, + Content: c, + State: st, + } } } @@ -296,19 +306,17 @@ func (ev *Evaluator) evalInclude(ast *IncludeAST) { files := splitSpaces(buf.String()) buf.Reset() for _, fn := range files { - now := time.Now().Unix() - f, err := os.Open(fn) + c, err := readFile(fn) if err != nil { if ast.op == "include" { Error(ev.filename, ev.lineno, fmt.Sprintf("%v\nNOTE: kati does not support generating missing makefiles", err)) } else { - ev.updateMakefileTimestamp(fn, -1) + ev.updateReadMakefile(fn, nil, FILE_NOT_EXISTS) continue } } - ev.updateMakefileTimestamp(fn, now) - defer f.Close() - err = ev.evalIncludeFile(fn, f) + ev.updateReadMakefile(fn, c, FILE_EXISTS) + err = ev.evalIncludeFile(fn, c) if err != nil { panic(err) } @@ -368,13 +376,10 @@ func (ev *Evaluator) eval(ast AST) { ast.eval(ev) } -func createTimestampArray(mp map[string]int64) []ReadMakefile { - var r []ReadMakefile - for k, v := range mp { - r = append(r, ReadMakefile{ - Filename: k, - Timestamp: v, - }) +func createReadMakefileArray(mp map[string]*ReadMakefile) []*ReadMakefile { + var r []*ReadMakefile + for _, v := range mp { + r = append(r, v) } return r } @@ -399,6 +404,6 @@ func Eval(mk Makefile, vars Vars) (er *EvalResult, err error) { vars: ev.outVars, rules: ev.outRules, ruleVars: ev.outRuleVars, - readMks: createTimestampArray(ev.readMks), + readMks: createReadMakefileArray(ev.readMks), }, nil } diff --git a/fileutil.go b/fileutil.go index 80ad02e..2224d7d 100644 --- a/fileutil.go +++ b/fileutil.go @@ -1,6 +1,9 @@ package main -import "os" +import ( + "fmt" + "os" +) func exists(filename string) bool { f, err := os.Open(filename) @@ -10,3 +13,23 @@ func exists(filename string) bool { f.Close() return true } + +func readFile(filename string) ([]byte, error) { + f, err := os.Open(filename) + if err != nil { + return nil, err + } + fi, err := f.Stat() + if err != nil { + return nil, err + } + buf := make([]byte, fi.Size()) + n, err := f.Read(buf) + if err != nil { + return nil, err + } + if n != len(buf) { + return nil, fmt.Errorf("Unexpected file size: %d vs %d", n, len(buf)) + } + return buf, nil +} diff --git a/main.go b/main.go index d92456e..cfb516d 100644 --- a/main.go +++ b/main.go @@ -39,7 +39,7 @@ var ( type DepGraph struct { nodes []*DepNode vars Vars - readMks []ReadMakefile + readMks []*ReadMakefile isCached bool } @@ -150,8 +150,11 @@ func getDepGraph(clvars []string, targets []string) *DepGraph { bmk := getBootstrapMakefile(targets) - now := time.Now().Unix() - mk, err := ParseMakefile(makefile) + content, err := readFile(makefile) + if err != nil { + panic(err) + } + mk, err := ParseMakefile(content, makefile) if err != nil { panic(err) } @@ -207,11 +210,12 @@ func getDepGraph(clvars []string, targets []string) *DepGraph { panic(err2) } LogStats("dep build time: %q", time.Now().Sub(startTime)) - var readMks []ReadMakefile + var readMks []*ReadMakefile // Always put the root Makefile as the first element. - readMks = append(readMks, ReadMakefile{ - Filename: makefile, - Timestamp: now, + readMks = append(readMks, &ReadMakefile{ + Filename: makefile, + Content: content, + State: FILE_EXISTS, }) readMks = append(readMks, er.readMks...) return &DepGraph{ diff --git a/parser.go b/parser.go index 489f77f..b6ed84c 100644 --- a/parser.go +++ b/parser.go @@ -8,7 +8,6 @@ package main import ( "bufio" "bytes" - "errors" "fmt" "io" "os" @@ -574,6 +573,7 @@ func ParseMakefileFd(filename string, f *os.File) (Makefile, error) { return parser.parse() } +/* func ParseMakefile(filename string) (Makefile, error) { Log("ParseMakefile %q", filename) f, err := os.Open(filename) @@ -594,6 +594,7 @@ func ParseDefaultMakefile() (Makefile, string, error) { } return Makefile{}, "", errors.New("no targets specified and no makefile found.") } +*/ func GetDefaultMakefile() string { candidates := []string{"GNUmakefile", "makefile", "Makefile"} @@ -621,3 +622,9 @@ func ParseMakefileString(s string, name string, lineno int) (Makefile, error) { func ParseMakefileBytes(s []byte, name string, lineno int) (Makefile, error) { return parseMakefileReader(bytes.NewReader(s), name, lineno) } + +func ParseMakefile(s []byte, filename string) (Makefile, error) { + Log("ParseMakefile %q", filename) + parser := newParser(bytes.NewReader(s), filename) + return parser.parse() +} diff --git a/serialize.go b/serialize.go index f8f509d..4c5c8ef 100644 --- a/serialize.go +++ b/serialize.go @@ -94,7 +94,7 @@ type SerializableGraph struct { Tsvs []SerializableTargetSpecificVar Targets []string Roots []string - ReadMks []ReadMakefile + ReadMks []*ReadMakefile } func encGob(v interface{}) string { @@ -265,7 +265,7 @@ func DumpDepGraphCache(g *DepGraph, roots []string) { cacheFile := GetCacheFilename(g.readMks[0].Filename, roots) for _, mk := range g.readMks { // Inconsistent, do not dump this result. - if mk.Timestamp == -2 { + if mk.State == 2 { if exists(cacheFile) { os.Remove(cacheFile) } @@ -568,22 +568,20 @@ func LoadDepGraphCache(makefile string, roots []string) *DepGraph { g := LoadDepGraph(filename) for _, mk := range g.readMks { - // TODO: A hack for Android build. Maybe we should - // keep their contents instead of the timestamp. - if mk.Filename == "out/target/product/generic/clean_steps.mk" || - mk.Filename == "out/target/common/obj/previous_aidl_config.mk" || - mk.Filename == "out/target/product/generic/previous_build_config.mk" { - continue - } - - ts := getTimestamp(mk.Filename) - if mk.Timestamp >= 0 && ts < 0 { - LogAlways("Cache expired: %s", mk.Filename) - return nil + if mk.State != FILE_EXISTS && mk.State != FILE_NOT_EXISTS { + panic(fmt.Sprintf("Internal error: broken state: %d", mk.State)) } - if mk.Timestamp <= ts { - LogAlways("Cache expired: %s", mk.Filename) - return nil + if mk.State == FILE_NOT_EXISTS { + if exists(mk.Filename) { + LogAlways("Cache expired: %s", mk.Filename) + return nil + } + } else { + c, err := readFile(mk.Filename) + if err != nil || !bytes.Equal(c, mk.Content) { + LogAlways("Cache expired: %s", mk.Filename) + return nil + } } } g.isCached = true -- cgit v1.2.3