[MERGE] add stracing of specific functions support

John Arbash Meinel john at arbash-meinel.com
Fri Mar 23 14:18:45 GMT 2007


Robert Collins wrote:
> This is just a little helper to allow stracing of specific routines from
> python. I found the noise of python startup really annoying to wade
> through when profiling dirstate.
> 
> (It depends on the extended test suite support which is already approved
> but not merged till poolie unfreezes mainline).
> 
> -Rob
> 

+1 (conditional)

...

> +import os
> +import signal
> +import subprocess
> +import tempfile
> +

v- Is this test focused? Or is it going to be something like "bzr
--lsprof command" ?

> +# this is currently test-focused, so importing bzrlib.tests is ok. We might
> +# want to move feature to its own module though.
> +from bzrlib.tests import Feature

-- Extra space here

> +
> +def strace(function, *args, **kwargs):
> +    """Invoke strace on function.
> +
> +    :return: A StraceResult.
> +    """

Do you want to just return an StraceResult? It seems like it would be
more useful to also return the return value of the function.

Also, I've found that it would be useful if we could give an --lsprof
even if there is an exception raised. And I assume it would be the same
here. So possibly we could do:

result = exc = None
try:
  result = function(*args, **kwargs)
except Exception, e:
  exc = e

...

return (StraceResult(log), result, exc)

> +    # capture strace output to a file
> +    log_file = tempfile.TemporaryFile()
> +    log_file_fd = log_file.fileno()
> +    pid = os.getpid()
> +    # start strace
> +    proc = subprocess.Popen(['strace',
> +        '-f', '-r', '-tt', '-p', str(pid),
> +        ],
> +        stderr=log_file_fd,
> +        stdout=log_file_fd)
> +    # TODO? confirm its started (test suite should be sufficient)
> +    # (can loop on proc.pid, but that may not indicate started and attached.)
> +    function(*args, **kwargs)
> +    # stop strace
> +    os.kill(proc.pid, signal.SIGQUIT)
> +    proc.communicate()
> +    # grab the log
> +    log_file.seek(0)
> +    log = log_file.read()
> +    log_file.close()
> +    return StraceResult(log)

...

I'm pretty strong on being able to get the return value of the function,
not so much about the exception. And I'm otherwise +1 on having this
included.

John
=:->



More information about the bazaar mailing list