[MERGE] Extend -Dhpss to emit a count of HPSS calls to stderr

[John Arbash Meinel]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Andrew Bennetts wrote:
> This patch extends the -Dhpss option to dump a count of HPSS calls to
> stderr for each SmartClientMedium that was used.
> 
> Example output:
> 
> $ ./bzr revno bzr://localhost/branch -Dhpss
> 3731
> HPSS calls: 8 (<bzrlib.smart.medium.SmartTCPClientMedium object at 0x8565dac>)
> 
> I know that it's a bit unusual for a debug flag to cause output
> somewhere other than the log file, but I think it's helpful in this
> case.  I'm open changing this behaviour, or to making a new debug flag
> for this separate to the existing -Dhpss, but so far I think I like it
> this way.

I'll mention that "-Dmemory" writes out using "trace.note()" which both
writes to sys.stderr *and* to .bzr.log, which I think is a good way to
do it. It means you can capture the value for long term, and see it
right away.

...

> The main drawback to this code is that when -Dhpss is used, it will keep
> adding hook functions for each SmartClientMedium that is instantiated.
> Those hook functions are never unregistered because there's no API to do
> so.  It'd probably be good to fix that, but I don't think it's a big
> deal for debugging code.

I think it is something we should do for the Hook api (something like
uninstall_name_hook()). Originally the hook api was simply a dict of
lists, so you would just call 'Branch.hooks['hook'].remove(func)', but I
think we've move away from that (which I feel is better anyway).

> 
> I have an alternative version of this patch (also attached, as a diff
> relative to this change, I hope it doesn't confuse BB), that installs a
> single hook function (rather than one per medium).  The hook function
> then had to find the matching weakref for the medium being used, which
> didn't seem as straightforward to me.  Also that approach conflicts more
> with the test suite, which resets hooks after each test, but this hook
> is only installed once per process.

I probably prefer the latter one. Instead of searching, you could use
the repr(ref) to index into a dict, or id(medium). You could even get
rid of the weakref code entirely by doing:

def __init__
  self.counts = {}

def track(self, medium):

  medium_info = (0, repr(medium))
  self.counts[id(medium)] = medium_info

That doesn't give you a 'done' callback, but we could just live with atexit.

John
=:->

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iEYEARECAAYFAkjbwWUACgkQJdeBCYSNAANv+wCdF5HLQNAq9hehG+bp31tO9p2m
fVoAnjkldDaFLGP3nPXBXEhrQ6kBCiaq
=+0ct
-----END PGP SIGNATURE-----