Remove filenames when showing function (default) granularity #110
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Previously we were showing filenames together with function names when generating reports at function granularity. This was an attempt to provide more information to the user, but it clutters the output. Also, in cases where a function is associated to multiple files (eg template instantiations in C++), keeping the filename may prevent us from combining nodes from the same function. Will stop keeping filenames at the default function granularity, and will remove the "functionnameonly" option which was previously providing this functionality.
aalexand
approved these changes
Mar 10, 2017
|
Yes, this may break some people. We could add a new granularity (eg -functionfile) to bring the old behavior back, but I'd rather only do that if there is a specific need. We should always discourage people from parsing pprof's text reports on their scripts/tests. |
aalexand
added a commit
to aalexand/pprof
that referenced
this pull request
Sep 20, 2018
…rity. This change consists of two relatively independent parts, but they are both about handling the granularity, so bundling them together. First, in google#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 google#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 google#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.
aalexand
added a commit
to aalexand/pprof
that referenced
this pull request
Sep 20, 2018
…rity. This change consists of two relatively independent parts, but they are both about handling the granularity, so bundling them together. First, in google#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 google#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 google#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.
aalexand
added a commit
to aalexand/pprof
that referenced
this pull request
Sep 20, 2018
…rity. This change consists of two relatively independent parts, but they are both about handling the granularity, so bundling them together. First, in google#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 google#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 google#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.
aalexand
added a commit
that referenced
this pull request
Sep 21, 2018
…rity. (#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.
gmarin13
pushed a commit
to gmarin13/pprof
that referenced
this pull request
Dec 17, 2020
…rity. (google#420) This change consists of two relatively independent parts, but they are both about handling the granularity, so bundling them together. First, in google#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 google#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 google#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.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Previously we were showing filenames together with function names
when generating reports at function granularity. This was an attempt
to provide more information to the user, but it clutters the output.
Also, in cases where a function is associated to multiple files
(eg template instantiations in C++), keeping the filename may prevent
us from combining nodes from the same function.
Will stop keeping filenames at the default function granularity, and will
remove the "functionnameonly" option which was previously providing this
functionality.
This fixes #106