Skip to content
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

Fix callgrind output compression #17

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

orivej
Copy link
Contributor

@orivej orivej commented Jul 14, 2019

callgrind format allows to compress repeated strings into one. yappi was using
the syntax for compression but did not compress much. This change makes yappi
actually compress repeated strings.

kcachegrind merges functions with the same compressed ID into one (as if they
were inlined) even if they were defined in different modules. Therefore this
change assigns different IDs to different functions with the same name.

@sumerc
Copy link
Owner

sumerc commented Jul 14, 2019

Thanks.
I will look into this in detail. BTW, I realized we do not have any callgrind format unit test check. Do we have any way of ensuring the callgrind output we generate is valid via simple command or library in unittests? Any idea on that?

@orivej
Copy link
Contributor Author

orivej commented Jul 14, 2019

We could assert that callgrind export of predefined stats exactly matches a reference export. To change callgrind export format one would have to update the reference text. You could either see that the change of the reference text is right, or open the old and the new reference in kcachegrind and evaluate the change.

@orivej orivej force-pushed the callgrind branch 3 times, most recently from 6353d78 to 4a01fdb Compare July 14, 2019 16:11
callgrind format allows to compress repeated strings into one.  yappi was using
the syntax for compression but did not compress much.  This change makes yappi
actually compress repeated strings.

kcachegrind merges functions with the same compressed ID into one (as if they
were inlined) even if they were defined in different modules. Therefore this
change assigns different IDs to different functions with the same name.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants