From 55fcd65fc8c7a63094ac6d858077edaa03bf5180 Mon Sep 17 00:00:00 2001 From: Raphael Vigee Date: Sun, 7 Apr 2024 22:28:52 +0100 Subject: [PATCH] Cleanup & fix nil dep --- scheduler/engine.go | 2 -- utils/xtypes/xtypes.go | 17 +++++++++++++++++ worker2/dep.go | 20 -------------------- worker2/dep_action.go | 4 ++-- worker2/dep_group.go | 4 ++-- worker2/deps.go | 4 ++-- worker2/deps_test.go | 38 +++++++++++++++----------------------- 7 files changed, 38 insertions(+), 51 deletions(-) diff --git a/scheduler/engine.go b/scheduler/engine.go index 81f36625..962c82fb 100644 --- a/scheduler/engine.go +++ b/scheduler/engine.go @@ -58,7 +58,6 @@ func (wgm *WaitGroupMap) All() worker2.Dep { defer wgm.mu.Unlock() wg := worker2.NewGroup() - wg.LinkDeps() for _, e := range wgm.m { wg.AddDep(e) @@ -80,7 +79,6 @@ func (wgm *WaitGroupMap) Get(s string) worker2.Dep { } wg := worker2.NewGroup() - wg.LinkDeps() wgm.m[s] = wg return wg diff --git a/utils/xtypes/xtypes.go b/utils/xtypes/xtypes.go index 17f96614..d8b3548e 100644 --- a/utils/xtypes/xtypes.go +++ b/utils/xtypes/xtypes.go @@ -1,8 +1,25 @@ package xtypes +import "reflect" + // ForceCast force casting any type by going through any // It is pretty somewhat unsafe, beware! func ForceCast[B any](a any) B { var b = a.(B) return b } + +// IsNil allows to go around nil interfaces +// see https://stackoverflow.com/a/78104852/3212099 +func IsNil(input interface{}) bool { + if input == nil { + return true + } + kind := reflect.ValueOf(input).Kind() + switch kind { + case reflect.Ptr, reflect.Map, reflect.Slice, reflect.Chan: + return reflect.ValueOf(input).IsNil() + default: + return false + } +} diff --git a/worker2/dep.go b/worker2/dep.go index 0e6ad554..76f60864 100644 --- a/worker2/dep.go +++ b/worker2/dep.go @@ -200,10 +200,6 @@ func (a *Action) Exec(ctx context.Context, ins InStore, outs OutStore) error { } func (a *Action) GetDepsObj() *Deps { - if a.deps == nil { - a.deps = NewDeps() - } - a.deps.setOwner(a) return a.deps } @@ -215,12 +211,6 @@ func (a *Action) DeepDo(f func(Dep)) { deepDo(a, f) } -func (a *Action) LinkDeps() { - for _, dep := range a.GetDepsObj().TransitiveDependencies() { - _ = dep.GetDepsObj() - } -} - type GroupConfig struct { Name string Deps []Dep @@ -238,17 +228,7 @@ func (g *Group) GetName() string { return g.name } -func (g *Group) LinkDeps() { - for _, dep := range g.GetDepsObj().TransitiveDependencies() { - _ = dep.GetDepsObj() - } -} - func (g *Group) GetDepsObj() *Deps { - if g.deps == nil { - g.deps = NewDeps() - } - g.deps.setOwner(g) return g.deps } diff --git a/worker2/dep_action.go b/worker2/dep_action.go index c133bd5a..d3c95ba3 100644 --- a/worker2/dep_action.go +++ b/worker2/dep_action.go @@ -5,8 +5,8 @@ type EventDeclared struct { } func NewAction(cfg ActionConfig) *Action { - a := &Action{} - _ = a.GetDepsObj() + a := &Action{deps: NewDeps()} + a.deps.setOwner(a) a.name = cfg.Name a.ctx = cfg.Ctx diff --git a/worker2/dep_group.go b/worker2/dep_group.go index 067b9437..3943c76e 100644 --- a/worker2/dep_group.go +++ b/worker2/dep_group.go @@ -5,8 +5,8 @@ func NewGroup(deps ...Dep) *Group { } func NewGroupWith(cfg GroupConfig) *Group { - g := &Group{} - _ = g.GetDepsObj() + g := &Group{deps: NewDeps()} + g.deps.setOwner(g) g.name = cfg.Name g.AddDep(cfg.Deps...) diff --git a/worker2/deps.go b/worker2/deps.go index 79043af7..e47fccbf 100644 --- a/worker2/deps.go +++ b/worker2/deps.go @@ -4,6 +4,7 @@ import ( "fmt" "github.com/hephbuild/heph/utils/ads" "github.com/hephbuild/heph/utils/sets" + "github.com/hephbuild/heph/utils/xtypes" "strings" "sync" ) @@ -96,7 +97,6 @@ func (d *Deps) flattenNamed(deps []Dep) []Dep { } return fdeps } - func (d *Deps) Add(deps ...Dep) { d.m.Lock() defer d.m.Unlock() @@ -106,7 +106,7 @@ func (d *Deps) Add(deps ...Dep) { } for _, dep := range deps { - if dep == nil { + if xtypes.IsNil(dep) { continue } if !d.has(dep) { diff --git a/worker2/deps_test.go b/worker2/deps_test.go index fbb68a75..2ec1ae7f 100644 --- a/worker2/deps_test.go +++ b/worker2/deps_test.go @@ -12,15 +12,11 @@ func s(s string) string { } func TestLink(t *testing.T) { - d1 := &Action{name: "1", deps: NewDeps()} - d2 := &Action{name: "2", deps: NewDeps(d1)} + d1 := NewAction(ActionConfig{Name: "1"}) + d2 := NewAction(ActionConfig{Name: "2", Deps: []Dep{d1}}) - d3 := &Action{name: "3", deps: NewDeps()} - d4 := &Action{name: "4", deps: NewDeps(d3)} - - for _, d := range []*Action{d1, d2, d3, d4} { - d.LinkDeps() - } + d3 := NewAction(ActionConfig{Name: "3"}) + d4 := NewAction(ActionConfig{Name: "4", Deps: []Dep{d3}}) assertDetached := func() { assert.Equal(t, s(` @@ -98,9 +94,9 @@ func TestLink(t *testing.T) { } func TestCycle1(t *testing.T) { - d1 := &Action{name: "1", deps: NewDeps()} - d2 := &Action{name: "2", deps: NewDeps(d1)} - d3 := &Action{name: "3", deps: NewDeps(d2)} + d1 := NewAction(ActionConfig{Name: "1"}) + d2 := NewAction(ActionConfig{Name: "2", Deps: []Dep{d1}}) + d3 := NewAction(ActionConfig{Name: "3", Deps: []Dep{d2}}) assert.PanicsWithValue(t, "cycle", func() { d2.AddDep(d3) @@ -108,11 +104,11 @@ func TestCycle1(t *testing.T) { } func TestCycle2(t *testing.T) { - d1 := &Action{name: "1", deps: NewDeps( /* d4 */ )} - d2 := &Action{name: "2", deps: NewDeps(d1)} + d1 := NewAction(ActionConfig{Name: "1", Deps: []Dep{ /* d4 */ }}) + d2 := NewAction(ActionConfig{Name: "2", Deps: []Dep{d1}}) - d3 := &Action{name: "3", deps: NewDeps()} - d4 := &Action{name: "4", deps: NewDeps(d3)} + d3 := NewAction(ActionConfig{Name: "3"}) + d4 := NewAction(ActionConfig{Name: "4", Deps: []Dep{d3}}) d1.AddDep(d4) @@ -122,23 +118,19 @@ func TestCycle2(t *testing.T) { } func TestRemoveStress(t *testing.T) { - root := &Action{name: "root", deps: NewDeps()} - root.LinkDeps() + root := NewAction(ActionConfig{Name: "root", Deps: []Dep{}}) for i := 0; i < 1000; i++ { - d := &Action{name: fmt.Sprint(i)} - d.LinkDeps() + d := NewAction(ActionConfig{Name: fmt.Sprint(i)}) root.AddDep(d) for j := 0; j < 1000; j++ { - d1 := &Action{name: fmt.Sprintf("%v-%v", i, j)} - d1.LinkDeps() + d1 := NewAction(ActionConfig{Name: fmt.Sprintf("%v-%v", i, j)}) d.AddDep(d1) } } - group := &Action{name: "group", deps: NewDeps(root)} - group.LinkDeps() + group := NewAction(ActionConfig{Name: "group", Deps: []Dep{root}}) group.GetDepsObj().Remove(root) }