diff --git a/go.mod b/go.mod index 52ce797..a5ea5c0 100644 --- a/go.mod +++ b/go.mod @@ -4,7 +4,6 @@ go 1.21 require ( github.com/chzyer/readline v1.5.0 - github.com/golang-collections/collections v0.0.0-20130729185459-604e922904d3 github.com/muesli/reflow v0.3.0 github.com/prataprc/goparsec v0.0.0-20211219142520-daac0e635e7e github.com/spf13/cobra v1.8.0 diff --git a/go.sum b/go.sum index c82c93a..999af97 100644 --- a/go.sum +++ b/go.sum @@ -27,8 +27,6 @@ github.com/go-logr/logr v1.3.0 h1:2y3SDp0ZXuc6/cjLSZ+Q3ir+QB9T/iG5yYRXqsagWSY= github.com/go-logr/logr v1.3.0/go.mod h1:9T104GzyrTigFIr8wt5mBrctHMim0Nb2HLGrmQ40KvY= github.com/go-logr/stdr v1.2.2 h1:hSWxHoqTgW2S2qGc0LTAI563KZ5YKYRhT3MFKZMbjag= github.com/go-logr/stdr v1.2.2/go.mod h1:mMo/vtBO5dYbehREoey6XUKy/eSumjCCveDpRre4VKE= -github.com/golang-collections/collections v0.0.0-20130729185459-604e922904d3 h1:zN2lZNZRflqFyxVaTIU61KNKQ9C0055u9CAfpmqUvo4= -github.com/golang-collections/collections v0.0.0-20130729185459-604e922904d3/go.mod h1:nPpo7qLxd6XL3hWJG/O60sR8ZKfMCiIoNap5GvD12KU= github.com/golang/glog v0.0.0-20160126235308-23def4e6c14b/go.mod h1:SBH7ygxi8pfUlaOkMMuAQtPIUF8ecWP5IEl/CR7VP2Q= github.com/golang/groupcache v0.0.0-20200121045136-8c9f03a8e57e/go.mod h1:cIg4eruTrX1D+g88fzRXU5OdNfaM+9IcxsU14FzY7Hc= github.com/golang/groupcache v0.0.0-20210331224755-41bb18bfe9da h1:oI5xCqsCo564l8iNU+DwB5epxmsaqB+rhGL0m5jtYqE= diff --git a/lisp/env.go b/lisp/env.go index d57a46b..c8aba86 100644 --- a/lisp/env.go +++ b/lisp/env.go @@ -867,8 +867,8 @@ func (env *LEnv) EvalSExpr(s *LVal) *LVal { args := call args.Cells = args.Cells[1:] if env.Runtime.Profiler != nil { - env.Runtime.Profiler.Start(fun) - defer env.Runtime.Profiler.End(fun) + stop := env.Runtime.Profiler.Start(fun) + defer stop() } switch fun.FunType { case LFunNone: diff --git a/lisp/profiler.go b/lisp/profiler.go index a50cf2e..518a70c 100644 --- a/lisp/profiler.go +++ b/lisp/profiler.go @@ -4,17 +4,14 @@ const ElpsVersion = "1.7" // Interface for a profiler type Profiler interface { - // Is the profiler enabled? + // IsEnabled determines if the profiler enabled. IsEnabled() bool - // Enable the profiler + // Enable enables the profiler Enable() error - // Set the file to output to + // SetFile sets the file to output to SetFile(filename string) error - // End the profiling session and output summary lines + // Complete ends the profiling session and output summary lines Complete() error - // Marks the start of a process - Start(function *LVal) - // Marks the end of a process - End(function *LVal) + // Start the start of a process, and returns a function to stop. + Start(function *LVal) func() } - diff --git a/lisp/x/profiler/callgrind.go b/lisp/x/profiler/callgrind.go index fec2ed8..68bcbb8 100644 --- a/lisp/x/profiler/callgrind.go +++ b/lisp/x/profiler/callgrind.go @@ -126,14 +126,14 @@ func (p *callgrindProfiler) getRef(name string) string { return fmt.Sprintf("(%d) %s", p.refCounter, name) } -func (p *callgrindProfiler) Start(function *lisp.LVal) { +func (p *callgrindProfiler) Start(function *lisp.LVal) func() { if !p.enabled { - return + return func() {} } switch function.Type { case lisp.LInt, lisp.LString, lisp.LFloat, lisp.LBytes, lisp.LError, lisp.LArray, lisp.LQuote, lisp.LNative, lisp.LQSymbol, lisp.LSortMap: // We don't need to profile these types. We could, but we're not that LISP :D - return + return func() {} case lisp.LFun, lisp.LSymbol, lisp.LSExpr: // Mark the time and point of entry. It feels like we're building the call stack in Runtime // again, but we're not - it's called, not callers. @@ -142,6 +142,10 @@ func (p *callgrindProfiler) Start(function *lisp.LVal) { default: panic(fmt.Sprintf("Missing type %d", function.Type)) } + + return func() { + p.End(function) + } } // Generates a call ref so the same item can be located again diff --git a/lisp/x/profiler/go_annotator.go b/lisp/x/profiler/go_annotator.go index 7212d5e..41a1bbf 100644 --- a/lisp/x/profiler/go_annotator.go +++ b/lisp/x/profiler/go_annotator.go @@ -6,7 +6,6 @@ import ( "fmt" "runtime/pprof" - "github.com/golang-collections/collections/stack" "github.com/luthersystems/elps/lisp" ) @@ -19,14 +18,12 @@ type pprofAnnotator struct { runtime *lisp.Runtime enabled bool currentContext context.Context - contexts *stack.Stack } func NewPprofAnnotator(runtime *lisp.Runtime, parentContext context.Context) lisp.Profiler { return &pprofAnnotator{ runtime: runtime, currentContext: parentContext, - contexts: stack.New(), } } @@ -52,14 +49,15 @@ func (p *pprofAnnotator) Complete() error { return nil } -func (p *pprofAnnotator) Start(function *lisp.LVal) { +func (p *pprofAnnotator) Start(function *lisp.LVal) func() { if !p.enabled { - return + return func() {} } + oldContext := p.currentContext switch function.Type { case lisp.LInt, lisp.LString, lisp.LFloat, lisp.LBytes, lisp.LError, lisp.LArray, lisp.LQuote, lisp.LNative, lisp.LQSymbol, lisp.LSortMap: // We don't need to profile these types. We could, but we're not that LISP :D - return + return func() {} case lisp.LFun, lisp.LSymbol, lisp.LSExpr: // We're keeping the context on a stack here rather than using the pprof.Do function for the simple // reason that I would have had to make more changes to the lisp.EvalSExpr function to accommodate that, @@ -68,7 +66,7 @@ func (p *pprofAnnotator) Start(function *lisp.LVal) { // if we did it that way. fName := fmt.Sprintf("%s:%s", function.FunData().Package, getFunNameFromFID(p.runtime, function.FunData().FID)) labels := pprof.Labels("function", fName) - p.contexts.Push(p.currentContext) + oldContext = p.currentContext p.currentContext = pprof.WithLabels(p.currentContext, labels) // apply the selected labels to the current goroutine (NB this will propagate if the code branches further down... pprof.SetGoroutineLabels(p.currentContext) @@ -76,22 +74,7 @@ func (p *pprofAnnotator) Start(function *lisp.LVal) { panic(fmt.Sprintf("missing type %d", function.Type)) } -} - -func (p *pprofAnnotator) End(function *lisp.LVal) { - if !p.enabled { - return - } - switch function.Type { - case lisp.LInt, lisp.LString, lisp.LFloat, lisp.LBytes, lisp.LError, lisp.LArray, lisp.LQuote, lisp.LNative, lisp.LQSymbol, lisp.LSortMap: - // We don't need to profile these types. We could, but we're not that LISP :D - return - case lisp.LFun, lisp.LSymbol, lisp.LSExpr: - // And pop the current context back - p.currentContext = p.contexts.Pop().(context.Context) - default: - panic(fmt.Sprintf("missing type %d", function.Type)) - + return func() { + p.currentContext = oldContext } - } diff --git a/lisp/x/profiler/opencensus_annotator.go b/lisp/x/profiler/opencensus_annotator.go index 88816f5..5ba9550 100644 --- a/lisp/x/profiler/opencensus_annotator.go +++ b/lisp/x/profiler/opencensus_annotator.go @@ -5,7 +5,6 @@ import ( "errors" "fmt" - "github.com/golang-collections/collections/stack" "github.com/luthersystems/elps/lisp" "go.opencensus.io/trace" ) @@ -15,14 +14,12 @@ type ocAnnotator struct { enabled bool currentContext context.Context currentSpan *trace.Span - contexts *stack.Stack } func NewOpenCensusAnnotator(runtime *lisp.Runtime, parentContext context.Context) lisp.Profiler { return &ocAnnotator{ runtime: runtime, currentContext: parentContext, - contexts: stack.New(), } } @@ -60,33 +57,23 @@ func (p *ocAnnotator) Complete() error { return nil } -func (p *ocAnnotator) Start(function *lisp.LVal) { +func (p *ocAnnotator) Start(function *lisp.LVal) func() { if !p.enabled { - return + return func() {} } + oldContext := p.currentContext switch function.Type { case lisp.LInt, lisp.LString, lisp.LFloat, lisp.LBytes, lisp.LError, lisp.LArray, lisp.LQuote, lisp.LNative, lisp.LQSymbol, lisp.LSortMap: // We don't need to profile these types. We could, but we're not that LISP :D - return + return func() {} case lisp.LFun, lisp.LSymbol, lisp.LSExpr: fName := fmt.Sprintf("%s:%s", function.FunData().Package, getFunNameFromFID(p.runtime, function.FunData().FID)) - p.contexts.Push(p.currentContext) p.currentContext, p.currentSpan = trace.StartSpan(p.currentContext, fName) default: panic(fmt.Sprintf("missing type %d", function.Type)) } -} - -func (p *ocAnnotator) End(function *lisp.LVal) { - if !p.enabled { - return - } - switch function.Type { - case lisp.LInt, lisp.LString, lisp.LFloat, lisp.LBytes, lisp.LError, lisp.LArray, lisp.LQuote, lisp.LNative, lisp.LQSymbol, lisp.LSortMap: - // We don't need to profile these types. We could, but we're not that LISP :D - return - case lisp.LFun, lisp.LSymbol, lisp.LSExpr: + return func() { file, line := p.getSource(function) p.currentSpan.Annotate([]trace.Attribute{ trace.StringAttribute("file", file), @@ -94,11 +81,8 @@ func (p *ocAnnotator) End(function *lisp.LVal) { }, "source") p.currentSpan.End() // And pop the current context back - p.currentContext = p.contexts.Pop().(context.Context) + p.currentContext = oldContext p.currentSpan = trace.FromContext(p.currentContext) - default: - panic(fmt.Sprintf("Missing type %d", function.Type)) - } } diff --git a/lisp/x/profiler/opentelemetry_annotator.go b/lisp/x/profiler/opentelemetry_annotator.go index 01b6ac3..fea2d9d 100644 --- a/lisp/x/profiler/opentelemetry_annotator.go +++ b/lisp/x/profiler/opentelemetry_annotator.go @@ -5,7 +5,6 @@ import ( "errors" "fmt" - "github.com/golang-collections/collections/stack" "github.com/luthersystems/elps/lisp" "go.opentelemetry.io/otel" "go.opentelemetry.io/otel/attribute" @@ -22,14 +21,12 @@ type otelAnnotator struct { enabled bool currentContext context.Context currentSpan trace.Span - contexts *stack.Stack } func NewOpenTelemetryAnnotator(runtime *lisp.Runtime, parentContext context.Context) lisp.Profiler { return &otelAnnotator{ runtime: runtime, currentContext: parentContext, - contexts: stack.New(), } } @@ -75,41 +72,27 @@ func contextTracer(ctx context.Context) trace.Tracer { return otel.GetTracerProvider().Tracer(tracerName) } -func (p *otelAnnotator) Start(function *lisp.LVal) { +func (p *otelAnnotator) Start(function *lisp.LVal) func() { if !p.enabled { - return + return func() {} } + oldContext := p.currentContext switch function.Type { case lisp.LInt, lisp.LString, lisp.LFloat, lisp.LBytes, lisp.LError, lisp.LArray, lisp.LQuote, lisp.LNative, lisp.LQSymbol, lisp.LSortMap: // We don't need to profile these types. We could, but we're not that LISP :D - return + return func() {} case lisp.LFun, lisp.LSymbol, lisp.LSExpr: fName := fmt.Sprintf("%s:%s", function.FunData().Package, getFunNameFromFID(p.runtime, function.FunData().FID)) - p.contexts.Push(p.currentContext) p.currentContext, p.currentSpan = contextTracer(p.currentContext).Start(p.currentContext, fName) default: panic(fmt.Sprintf("missing type %d", function.Type)) } -} - -func (p *otelAnnotator) End(function *lisp.LVal) { - if !p.enabled { - return - } - switch function.Type { - case lisp.LInt, lisp.LString, lisp.LFloat, lisp.LBytes, lisp.LError, lisp.LArray, lisp.LQuote, lisp.LNative, lisp.LQSymbol, lisp.LSortMap: - // We don't need to profile these types. We could, but we're not that LISP :D - return - case lisp.LFun, lisp.LSymbol, lisp.LSExpr: + return func() { file, line := p.getSource(function) p.currentSpan.AddEvent("source", trace.WithAttributes(attribute.Key("file").String(file), attribute.Key("line").Int64(int64(line)))) p.currentSpan.End() - // And pop the current context back - p.currentContext = p.contexts.Pop().(context.Context) + p.currentContext = oldContext p.currentSpan = trace.SpanFromContext(p.currentContext) - default: - panic(fmt.Sprintf("Missing type %d", function.Type)) - } }