Merge lp:~jr/bzr-explorer/795034-refresh-branch-view-automatically into lp:bzr-explorer

[Jonathan Riddell]

Watch for changes in branches and refresh when they happen

[Andrew Bennetts (spiv)]

I think _find_file_watcher_paths could be written more simply as:

import os

    def _find_file_watcher_paths(self, root):
        result = [root]
        for dirpath, dirnames, filenames in os.walk(root):
            result.extend(os.path.join(dirpath, d) for d in dirnames)
        return result

At a minimum I'd suggest avoiding “fileList = fileList + …” in favour of fileList.extend(…) (or equivalently “fileList += …”), as I think the way you have written it CPython is likely to have to allocate a new list rather than growing the existing list in place, making it an O(n²) loop. Newer CPythons *might* optimise in this case, but I'm not sure they do.

It might be worth trying this on a large tree (maybe gcc or emacs?) just to verify there's no nasty performance surprises. But aside from that this seems fine to me, although I'm not a Qt or bzr-explorer expert.

[Andrew Bennetts (spiv)]

review: Approve

[John A Meinel (jameinel)]

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

On 6/16/2011 7:39 AM, Andrew Bennetts wrote:
> I think _find_file_watcher_paths could be written more simply as:
>
> import os
>
> def _find_file_watcher_paths(self, root):
> result = [root]
> for dirpath, dirnames, filenames in os.walk(root):
> result.extend(os.path.join(dirpath, d) for d in dirnames)
> return result
>
> At a minimum I'd suggest avoiding “fileList = fileList + …” in favour of fileList.extend(…) (or equivalently “fileList += …”), as I think the way you have written it CPython is likely to have to allocate a new list rather than growing the existing list in place, making it an O(n²) loop. Newer CPythons *might* optimise in this case, but I'm not sure they do.
>
> It might be worth trying this on a large tree (maybe gcc or emacs?) just to verify there's no nasty performance surprises. But aside from that this seems fine to me, although I'm not a Qt or bzr-explorer expert.

I'll note that bzrlib has "bzrlib.osutils._walkdirs_utf8" et al because
"os.walk()" is not particularly efficient. ISTR it stats every object 2
times at least. (It does something like

 if os.path.isdir():
  ...
 elif os.path.isfile():
  ...

Which means that the common case of everything is a file, everything
gets os.lstat() 2 times to determine that. Should obviously be fast
because you just stat'd it, but it is still a syscall.

The most straightforward api is "osutils.walkdirs()" but _walkdirs_utf8
is more efficient for filesystems like Linux where we think about them
in terms of Unicode, but the actual paths on disk are in 8-bit (utf-8).

Because you only care about directories, you could actually just do:

result = [root]
for (dir_info, dir_entries) in osutils._walkdirs_utf8(root):
  utf8_relpath, system_path = dir_info
  result.append(system_path)
return result

Check and make sure, but we should descend into every directory, which
means that you can just use the "dir_info" portion.

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

iEYEARECAAYFAk35s+IACgkQJdeBCYSNAAOuMgCffOJ9zTy0kBdQfGPVSp3NW+dr
+loAniV8HjoY2pRFnVsdh1vtToQ0jNtA
=/Fp5
-----END PGP SIGNATURE-----

[lp:~jr/bzr-explorer/795034-refresh-branch-view-automatically]

518. By Jonathan Riddell on 2011-06-16

simplify _find_file_watcher_paths()