[MERGE] lazy inventory writes

[John Arbash Meinel]

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

Aaron Bentley wrote:
> Robert Collins wrote:
>>> WorkingTree currently writes the inventory on nearly every mutation
>>> operation. This patch adds the facility for us to write it only on
>>> unlock, and updates set_root_id to use this. This combined with a minor
>>> change to tree initialisation gives us only one inventory write to disk
>>> during tree creation.
> 
> This sounds reasonable.  +1 except for two things:
> 
>>> @@ -1018,7 +1054,7 @@
>>>                              ["rename rolled back"])
>>>          except:
>>>              # restore the inventory on error
>>> -            self._set_inventory(orig_inv)
>>> +            self._set_inventory(orig_inv, dirty=original_modified)
>>>              raise
>>>          self._write_inventory(inv)
> 
> Shouldn't this be removed?

Ultimately, the dirty flag should me set as part of this call, rather
than writing out the inventory at the end. But Robert wasn't tracking
down all of the write_inventory locations just yet. He was starting with
a couple that had to do with the specific thing at hand. add, remove,
rename_one & move, and others all modify the inventory, and should be
changed so that they set the dirty flag rather than doing a read,
modify, write.

This is just a step along that way, not a complete conversion.


> 
>>> @@ -1555,16 +1600,31 @@
>>>      @needs_tree_write_lock
>>>      def set_root_id(self, file_id):
>>>          """Set the root id for this tree."""
>>> -        inv = self.read_working_inventory()
>>> +        # for compatability 
>>> +        if file_id is None:
>>> +            symbol_versioning.warn(symbol_versioning.zero_twelve
>>> +                % 'WorkingTree.set_root_id with fileid=None',
>>> +                DeprecationWarning,
>>> +                stacklevel=3)
>>> +            file_id = ROOT_ID
>>> +        inv = self._inventory
> 
> How do we know self._inventory isn't None?

At this point, self._inventory is always read. (As part of the
constructor). The final goal is to make self._inventory not exist, and
in its place have a call which will only work inside a transaction, and
then only maintain the inventory state for the duration of a read or
write lock.

> 
> And yes, I agree we should deprecate setting root_id to None.
> 
> Aaron

Yeah, it used to work as a side effect of something else. But in case it
was really happening, we went for deprecating it.

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

iD8DBQFFKuPrJdeBCYSNAAMRAr8OAKDENukD76/Jiexj7tEZ4Qn3MjnE6QCgzhx+
kTvUZRlrMPkh3wHTrhdaLVE=
=ruzp
-----END PGP SIGNATURE-----