Code review comment for lp:~jelmer/bzr/export-use-tree-timestamp

> Jelmer Vernooij wrote:
> > Jelmer Vernooij has proposed merging lp:~jelmer/bzr/export-use-tree-
> timestamp into lp:bzr.
> >
> > Requested reviews:
> > bzr-core (bzr-core)
> >
> >
> > This adds an extra argument ``use_tree_timestamp`` to the exporters, to use
> the commit time of the commit in which a file revision was introduced as the
> mtime when exporting. This is useful as a means of getting deterministic
> exports (i.e. the generated tarball from a particular revision is always the
> same).
> >
> > use_tree_timestamps is not the default for performance reasons, but the
> provided infrastructure is useful for bzr-builddeb.
> >
>
> Note that using 'tree.get_file_mtime()' for each file is a bit of a
> shame. Specifically, the api doesn't really do any caching (and it isn't
> very easy to do so).
>
> If we wanted it to perform well, doing 1 pass over the inventory to find
> revisions we will care about, doing another request to read all of those
> revisions in 1 pass, should actually be pretty decent. (log can display
> all revisions in generally a rather decent amount of time, this should
> be no slower than that.)
Yeah,I realize that. That seemed like significantly more work though so it's something I'd rather do in a separate patch.

This at least adds the functionality, but doesn't have any performance consequences for the default behaviour. The overhead at the moment doesn't seem too bad. On a Samba tree with a warm cache:

./bzr export x.tar.gz ~/src/samba/trunk 15.58s user 0.28s system 96% cpu 16.374 total
./bzr export --per-file-timestamps x.tar.gz ~/src/samba/trunk 19.06s user 0.43s system 86% cpu 22.629 total
./bzr ls --versioned --recursive ~/src/samba/trunk | wc -l
7921

Cheers,

Jelmer