[MERGE] 'bzr pack' tells the index builder to optimize

[John Arbash Meinel]

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

Andrew Bennetts wrote:
> bb:tweak
> 
> The tests feel slightly weak, because they are mainly about testing that
> a flag is or isn't set on an object, rather than testing that
> for_size=True and friends actually cause a change in behaviour.  I'm
> don't think it's easy fix that, so I'm fine with this, but I felt I
> ought to mention it.

Well, I considered that we could create a btree format repository, and
then check the size of the index. If we force known data into it, we
should be able to craft something that will pack better with the
advanced settings. That said, it is going to be a bit voodoo, and I
wouldn't want to expect that zlib will produce exactly the same bytes
across all platforms.

So if we made the data nearly trivially redundant (keys of all the same
character, etc), then I know the old form would only minimally compress
them, and the new form would pack more per page. It still feels like a
"soft" test to me, do you want me to put in the effort?

> 
> Otherwise, just one comment:
> 
> John Arbash Meinel wrote:
> [...]
>> === modified file 'bzrlib/repofmt/pack_repo.py'
>> --- bzrlib/repofmt/pack_repo.py	2008-10-01 05:40:45 +0000
>> +++ bzrlib/repofmt/pack_repo.py	2008-10-16 18:58:22 +0000
>> @@ -971,6 +971,21 @@
>>          # TODO: combine requests in the same index that are in ascending order.
>>          return total, requests
>>  
>> +    def open_pack(self):
>> +        """Open a pack for the pack we are creating."""
>> +        new_pack = NewPack(self._pack_collection._upload_transport,
>> +            self._pack_collection._index_transport,
>> +            self._pack_collection._pack_transport, upload_suffix=self.suffix,
>> +            file_mode=self._pack_collection.repo.bzrdir._get_file_mode(),
>> +            index_builder_class=self._pack_collection._index_builder_class,
>> +            index_class=self._pack_collection._index_class)
>> +
>> +        new_pack.revision_index.set_optimize(for_size=True)
>> +        new_pack.inventory_index.set_optimize(for_size=True)
>> +        new_pack.text_index.set_optimize(for_size=True)
>> +        new_pack.signature_index.set_optimize(for_size=True)
>> +        return new_pack
>> +
> 
> That's a lot of duplication.  Why not up-call to the parent class?  i.e.
> implement this as:
> 
>     def open_pack(self):
>         """Open a pack for the pack we are creating."""
>         new_pack = super(OptimisingPacker, self).open_pack()
>         new_pack.revision_index.set_optimize(for_size=True)
>         new_pack.inventory_index.set_optimize(for_size=True)
>         new_pack.text_index.set_optimize(for_size=True)
>         new_pack.signature_index.set_optimize(for_size=True)
>         return new_pack
>         
> -Andrew.
> 
> 

Thanks for the catch, I think I meant to do that, and ended up
copy&pasting instead.

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

iEYEARECAAYFAkj/faYACgkQJdeBCYSNAAPARQCdEdyXZlDEhI5MEk90TW7cgfGQ
bjIAn3u5DF3Y9Ipa016Vpi6zVWoqkX4+
=+eOq
-----END PGP SIGNATURE-----