Skip to content

Commit

Permalink
Remove Stack from profiler
Browse files Browse the repository at this point in the history
  • Loading branch information
sam-at-luther committed Dec 16, 2023
1 parent e96c3b6 commit 27d3a15
Show file tree
Hide file tree
Showing 8 changed files with 34 additions and 86 deletions.
1 change: 0 additions & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 0 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -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=
Expand Down
4 changes: 2 additions & 2 deletions lisp/env.go
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
15 changes: 6 additions & 9 deletions lisp/profiler.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}

10 changes: 7 additions & 3 deletions lisp/x/profiler/callgrind.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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
Expand Down
31 changes: 7 additions & 24 deletions lisp/x/profiler/go_annotator.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import (
"fmt"
"runtime/pprof"

"github.com/golang-collections/collections/stack"
"github.com/luthersystems/elps/lisp"
)

Expand All @@ -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(),
}
}

Expand All @@ -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,
Expand All @@ -68,30 +66,15 @@ 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)
default:
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
}

}
28 changes: 6 additions & 22 deletions lisp/x/profiler/opencensus_annotator.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import (
"errors"
"fmt"

"github.com/golang-collections/collections/stack"
"github.com/luthersystems/elps/lisp"
"go.opencensus.io/trace"
)
Expand All @@ -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(),
}
}

Expand Down Expand Up @@ -60,45 +57,32 @@ 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),
trace.Int64Attribute("line", int64(line)),
}, "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))

}
}

Expand Down
29 changes: 6 additions & 23 deletions lisp/x/profiler/opentelemetry_annotator.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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(),
}
}

Expand Down Expand Up @@ -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))

}
}

Expand Down

0 comments on commit 27d3a15

Please sign in to comment.