-
Notifications
You must be signed in to change notification settings - Fork 66
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support pprof profiling in BadWolf #155
Conversation
4af6505
to
ec76870
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A couple of small comments on indenting the error flow so it is more clear to the reader.
I'll go ahead and approve but PTAL at them before submitting.
@@ -175,6 +220,59 @@ func REPL(od storage.Store, input *os.File, rl ReadLiner, chanSize, bulkSize, bu | |||
done <- false | |||
continue | |||
} | |||
if strings.HasPrefix(l, "start profiling") { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For a separate PR/Unralated to your changes:
Consider updating the loop to have a switch instead of several if strings.HasPrefix
checks.
Remember: in Go, switch statements are more general than in other languages, so you can have cases checking for HasPrefix
.
https://golang.org/doc/effective_go.html#switch
Besides being more readable, it can probably simplify the done <- false/continue
repetitions. You might want to look at labels to control the flow here, rather than using the channel :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you Tati! It is cool that switch
can be this general in Go, we will keep this in mind for a future PR as well. =]
… appended to the same files
This PR comes to allow the user to have more visibility on latency and memory metrics when using BadWolf, adding support for pprof profiling through BadWolf's CLI.
When in the terminal, after already entering the BadWolf's CLI with the command
bw bql
, the user can now write:To start
pprof
profiling. The user can also specify the CPU profiling rate with the-cpurate
flag below:Similarly, to stop it they can just write:
With profiling enabled, the metrics for latency and memory consumption for the BQLs executed will be printed into two files,
cpuprofile
andmemprofile
, that can then be consumed and visualized through thego pprof
CLI tool.Commands such as
go tool pprof -http=":8081" bw cpuprofile
open in the browser an interface to visualize the CPU metrics: the user can see the graph of function calls made and the latencies involved, useful to identify bottlenecks; they can visualize the correspondent Flame Graph as well, and also an ordered list of the heaviest function calls made. Another useful command isgo tool pprof -pdf bw cpuprofile > cpuprofile.pdf
to generate pdfs from the profiling files. Withmemprofile
the previouspprof
commands work similarly.