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

Add timing information on traced function #34

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

Conversation

mistotebe
Copy link

q is very helpful when zeroing in on performance/timeout issues as well, make it even better by explicitly logging the time spent in the function.

@mithro
Copy link
Collaborator

mithro commented Oct 14, 2015

Hi @mistotebe

Thanks for your patch! It looks like it currently isn't pep8 clean, can you please fix that?

=== Running pep8 on files
pep8 q.py setup.py test/test_basic.py
q.py:287:80: E501 line too long (96 > 79 characters)
q.py:305:80: E501 line too long (92 > 79 characters)
make: *** [pep8] Error 1

You should be able to run a make pep8 in the q directory (or even better yet a make all which will run the tests and build too!).

@mistotebe
Copy link
Author

Hi @mithro, I've pushed an updated version. I ran make test, did not notice there being a pep8 target. Should be clean now.

@mithro
Copy link
Collaborator

mithro commented Oct 14, 2015

Generally looks good. Can you think of any way to add a decent test for this functionality?

@mistotebe
Copy link
Author

It shows you the timing so probably not, I could try adding a sleep() to it and check that it shows at least as much. Not sure.

@mistotebe
Copy link
Author

There is an alternative approach to the logging that might actually be even more useful (potentially even replacing the "insert a blank line if we had more than 5 s between messages" bit):

  • for each message, log time since last message e.g.: 2.3s(+0.9s)
  • upon leaving the trace(), log time spent in that function e.g.: 2.3s(+0.9s) --> 1.2s my_function()

@zestyping zestyping force-pushed the master branch 2 times, most recently from 56ddf51 to d9f393c Compare October 1, 2019 10:05
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