Make tag filter of the form `-tagfilter=foo=1` work. (#509)
Tag filters like `-tagfilter=foo=1` currently cannot be used to filter
by a unitless numeric tag as the tag filter regular expression expects
the unit part to be always present. They can be made working by using
`-tagfilter=foo=1unit` instead, but that's weird syntax. So fix this by
merely making the unit part optional.
I was wondering if this simple fix could have any unintended effects but
as far as I can tell from testing it does not.
Make '-noinlines' a separate flag, introduce '-filefunctions' granularity. (#420)
This change consists of two relatively independent parts, but they are
both about handling the granularity, so bundling them together.
First, in #415 there is a discussion that having a granularity mode by
source lines but with inlines hidden would be useful. The agreement is
also that adding `-linenoinlines` granularity would make the granularity
flags too messy (and they are already somewhat messy with `-addresses`
and `-addressnoinlines`. So, it was proposed to make `-noinlines` a
separate flag, which is what this change does. Note that the flag is now
pulled out of the granularity group so it's a bit of backward
incompatible change but I think it is acceptable. For the example in
issue #415 the user would now be able to specify `-list foo -noinlines`
to produce annotated source where the metrics from the inlined functions
are attributed to the calling inliner line.
With this change, I am also dropping the `-addressnoinlines` granularity
which is now supported as `-addresses -noinlines` combination. I
couldn't find any usage of this option at least internally at Google, so
I think it's safe to remove it.
Second, I am adding a separate `-filefunctions` granularity which groups
the data by both function and file. This is a follow-up to #110 from the
past where we changed the `-functions` granularity to not group by file
(it used to), and since then there was a couple of reports where using
just function name alone would over-aggregate the data in cases when a
function with the same name is contained in multiple source files (e.g.
see b/18874275 internally).
Also, make a number of assorted documentation and `-help` fixes.
* make filters applied to -proto output
* update comments
* address comments
* reassign flags in TestParse
* update TestParse to address comments
* use original names for options in TestParse
internal/driver: fix new unkeyed composite lit vet warnings (#298)
The current version of go vet does not flag these as noted in
golang/go#23539. A fix is in the works, and will likely be included in
Go 1.11. Make the updated vet tool happy, so that the vendored copy of
pprof in the Go tree can be updated and make the vet trybot happy.
Fixes #297.
* Add support for tag units
Updated tests for adding tag units
* updated proto docs to describe tag units
* fixed formatting
* added additional test
* require specific units for graph tag to be recognixed
* updated tests and formatting
* Fixed formatting
* Fixed style error
* modify proto_test to be sure tag units not overwritten
* Clarified label docs
* call legacy profile memory allocations size, not byte
* updated tests to call bytes tags in legacy profile size
* Revert "updated tests to call bytes tags in legacy profile size"
This reverts commit 9289378af2.
* Revert "call legacy profile memory allocations size, not byte"
This reverts commit 6b18973562.
* Updated so units not modified when profile written out
* Fixed formatting errors
* Removed field for inferred numeric label units from profile
* Modified profile String() output
* fixed formatting error
* changed name of function used to identify units for numeric labels
* Refactor String() for profile
* Modified for clarity and address comments
* renamed function for clarity
* Made numeric label units field seperate in profile
* Modified test
* Refactored identifyNumLabelUnits()
* Updated comments
* Fixed formatting error
* Addressed comments
* Fixed style error
* clarify comment
* Refactored to address comments
* addressed comments
* addressed comments
* addressed comments
Stop treating -symbolize=force as the default (#210)
* Stop treating -symbolize=force as the default
The code parsing symbolize options was treating the empty option as
"force", effectively making -symbolize=force the default. This would
unnecessarily symbolize profiles that were already symbolized.
* Add more testing for symbolize option parsing
Do all parsing of symbolize options on a single function and
do more thorough testing of it.
* Have symbolz honor -force
Currently -symbolize=remote will force remote symbolization of mappings
that already had mappings. Making that depend on an explicit
-symbolize=force to make it consistent with local symbolization.
* Applied fixes from latest review iteration
* Add comment on symbolz check for resymbolization
* Reformatting comment
* Modified variable type to have []string value
* added repeatableStringKind
* modify repeatableStringKind to actually handle multiple values
* Modify applyFocus to work with multiple values for tagfocus/tagignore
* Modify applyFocus to work with multiple values for tagfocus/tagignore
* Modified CompileTagFilter to handle tag filters of the form -tagfocus=key1=val1
* Modified StringList
* add testing for compileTagFilter
* fixed typo
* fixed code style error
* Revert "fixed code style error"
This reverts commit 0b1826c163.
* Revert "fixed typo"
This reverts commit 2593652bd0.
* Revert "add testing for compileTagFilter"
This reverts commit 152d7b0363.
* Revert "Modified StringList"
This reverts commit b57d35a762.
* Revert "Modified CompileTagFilter to handle tag filters of the form -tagfocus=key1=val1"
This reverts commit ebe1e344bf.
* Revert "Modify applyFocus to work with multiple values for tagfocus/tagignore"
This reverts commit 3b62091b67.
* Revert "Modify applyFocus to work with multiple values for tagfocus/tagignore"
This reverts commit 7172159cd3.
* Revert "modify repeatableStringKind to actually handle multiple values"
This reverts commit abb1e3ec4b.
* Revert "added repeatableStringKind"
This reverts commit 8b8e4e76fc.
* Revert "Modified variable type to have []string value"
This reverts commit 5f213dc4d1.
* added support for filtering by specific tags
* Fixed formatting problems
* Updated variable name for clarity
* Added test cases for tag filtering
* Update tagfocus and tagignore description
* Fixed formatting error
* Updated docs to explain new tag filtering
* Updated docs to clarify tag filtering
* Updated docs to clarify tag filtering
* Updated docs and help comments for tag filtering
* Updated docs for tag filtering
* Update pprof docs for tag filtering
* Update tagfocus and tagignore help descriptions
* Update tagfocus and tagignore descriptions in docs
* Update tagfocus and tagignore descriptions in docs and help
* Updated tagfocus and tagignore descriptions in docs; added TODO for regexp matching
* Updated tagfocus and tagignore descriptions in docs
* Modified docs fo tagfocus for clarity
* Align file names in weblist disassembly
The disassembly displayed by weblist attempts to align file:line
information after each instruction, but two things interfere with this
alignment: 1) the instruction text often has tabs in it, which defeats
the length-based alignment and generally interacts poorly with HTML
rendering and 2) the instruction text often has special HTML
characters like < and > in it, but we pad the string after escaping
it, so the length is again not representative of how it will display.
For example, the following shows what this looks like for disassembly
that contains < and >:
. . 41c634: cmp $0x200,%rdx mgcsweepbuf.go:130
. . 41c63b: jae 41c64f <runtime.(*gcSweepBuf).pop+0x4f> mgcsweepbuf.go:130
. . 41c63d: mov (%rax,%rdx,8),%rcx mgcsweepbuf.go:130
. . 41c64f: callq 424800 <runtime.panicindex> mgcsweepbuf.go:130
. . 41c654: ud2 mgcsweepbuf.go:130
Fix these problems by replacing tab characters with spaces and padding
the string before escaping it. After this, the file names are properly
aligned:
. . 41c634: cmp $0x200,%rdx mgcsweepbuf.go:130
. . 41c63b: jae 41c64f <runtime.(*gcSweepBuf).pop+0x4f> mgcsweepbuf.go:130
. . 41c63d: mov (%rax,%rdx,8),%rcx mgcsweepbuf.go:130
. . 41c64f: callq 424800 <runtime.panicindex> mgcsweepbuf.go:130
. . 41c654: ud2 mgcsweepbuf.go:130
* Separate discontiguous assembly blocks
Currently, weblist's disassembled output for each line simply lists
all instructions that are marked with that line number. This leads to
confusing disassembly because a lot of things look like straight-line
control flow when they aren't. For example, index panics look like
they always happen:
. . 41c62c: test %al,(%rax)
. . 41c62e: and $0x1ff,%edx
. . 41c634: cmp $0x200,%rdx
. . 41c63b: jae 41c64f <runtime.(*gcSweepBuf).pop+0x4f>
. . 41c63d: mov (%rax,%rdx,8),%rcx
. . 41c64f: callq 424800 <runtime.panicindex>
. . 41c654: ud2
In reality, 41c64f is at the end of the function, but it looks like we
call it immediately after a successful index operation.
Fix this by adding a vertical ellipses to separate blocks of assembly
that have other instructions between them. With this change, the above
looks like:
. . 41c62c: test %al,(%rax)
. . 41c62e: and $0x1ff,%edx
. . 41c634: cmp $0x200,%rdx
. . 41c63b: jae 41c64f <runtime.(*gcSweepBuf).pop+0x4f>
. . 41c63d: mov (%rax,%rdx,8),%rcx
⋮
. . 41c64f: callq 424800 <runtime.panicindex>
. . 41c654: ud2
Use a more regular mechanism to name sources during driver tests and
recognize the naming during fetching to avoid saving the test profiles
into $HOME/pprof.
Also move the https+insecure testing into fetch_test, to focus it on
fetching the profile.
This fixes #107
internal/report: change the format of tags command output
This change also includes:
- Instead of printing and sorting based on the first value in the
internal value slice, use the sample type user selected.
- Use the unit and print numbers in human friendly version.
- Add a flag '-update' to internal/driver/driver_test to update the
golden files.
TODO: apply to other tests that use other golden files.
Recognize (again) https+insecure scheme for symbolization POSTs. (#121)
* Recognize (again) https+insecure scheme for symbolization POSTs.
This reverts the revert of #111 relaxing the test to make it pass as the
issue reported in #111 is caused by unrelated #120.
Added a test, verified the test failed before the fix.
This fixes #94.
* Fix the staticcheck failure.
*: golint, go tool vet, unsued, gosimple, staticcheck (#113)
* `git ls-files -- '*.go' | xargs gofmt -s -w`
* driver: unexport InternalOptions
This was an exported method that returned an internal type, so it's
unlikely that anyone depends on its presence.
* internal/binutils: remove newline in error string
golint reported: error strings should not be capitalized or end with
punctuation or a newline
* internal/driver: s/buildId/buildID/ per golint
* internal/elfexec: remove punctuation in error string
golint reported: error strings should not be capitalized or end with
punctuation or a newline
* internal/graph: correct comment
Found with golint.
* internal/graph: add method comment
golint reported: exported method Edge.WeightValue should have comment
or be unexported
* internal/report: remove newline in error string
golint reported: error strings should not be capitalized or end with
punctuation or a newline
* *: remove unused code
Found with honnef.co/go/tools/cmd/unused:
internal/binutils/disasm_test.go:137:8: const objdump is unused (U1000)
internal/driver/driver_test.go:353:6: type testProfile is unused (U1000)
internal/graph/graph.go:838:6: func countEdges is unused (U1000)
profile/proto.go:148:6: func encodeStringOpt is unused (U1000)
* *: simply code
Found with honnef.co/go/tools/cmd/gosimple:
internal/driver/commands.go:471:24: should omit comparison to bool constant, can be simplified to !b (S1002)
internal/driver/driver.go:163:5: should omit comparison to bool constant, can be simplified to !trim (S1002)
internal/driver/driver.go:168:5: should omit comparison to bool constant, can be simplified to !focus (S1002)
internal/driver/driver.go:172:5: should omit comparison to bool constant, can be simplified to !tagfocus (S1002)
internal/driver/driver.go:176:5: should omit comparison to bool constant, can be simplified to !hide (S1002)
internal/graph/dotgraph.go:213:34: should use make(map[string][]*Tag) instead (S1019)
internal/report/report.go:1061:3: should replace loop with label = append(label, rpt.options.ProfileLabels...) (S1011)
profile/proto.go:157:5: should omit comparison to bool constant, can be simplified to !x (S1002)
* *: correct mistakes
Found with honnef.co/go/tools/cmd/staticcheck:
driver/driver.go:280:20: infinite recursive call (SA5007)
internal/driver/driver_focus.go:54:11: this value of err is never used (SA4006)
profile/proto.go:74:32: x | 0 always equals x (SA4016)
* Add linters to .travis.yml
Do not honor call_tree option when generating non-visual reports.
There is a check to only honor the call_tree option when generating non-visual
reports, but it wasn't being checked in all appropriate places, causing a
mismatch that triggered a panic.
Previous loc information on assembly listing was being printed for
every line, while the intent was to print it only when it changes.
Also update the tests to expose and test that case, and remaster
other tests to match.
Disassembly reports generated by pprof -disasm will now include line number
information as generated by objdump. This will make the generated assembly more
readable.
As part of this I've introduced a new assemblyInstruction struct. Previously
the code was reusing the graph.Node to represent assembly instructions but it
seems better to have a dedicated type for this.
When generating callgrind format output, produce cost lines at
instruction granularity. This allows visualizers supporting the
callgrind format to display instruction-level profiling information.
We also need to provide the object file (ob=) in order for tools to find
the object file to disassemble when displaying assembly.
We opportunistically group cost lines corressponding to the same
function together, reducing the number of superfluous description lines.
Subposition compression (relative position numbering) is also used to
reduce the output size.
Garrett Wang