[MERGE] Faster LocalTransport.put() and append() methods.

[Robert Collins]

On Sat, 2006-06-10 at 08:10 -0500, John Arbash Meinel wrote:
> Robert Collins wrote:
> ...
> 
> >> We should at least be 'chmod' ing if we create the file, see my earlier
> >> discussion about shared branches. And we need 'chmod' because of umask.
> >> Either that, or we always run with umask '0000', which I think is worse.
> > 
> > I dont see why we need to chmod *except* when making a branch new file
> > on disk.
> > 
> > We currently chmod in the following cases when a mode is specified...
> > AtomicFile
> > LocalTransport.append() with a new file
> > LocalTransport.append() with an existing file
> > 
> > Thats for .bzr files...
> 
> I agree. We should only need to do so when creating a new file, but how
> do we detect that?

Open the file with os.open and call os.seek - we have to anyway to
return the offset. If its 0 we treat it as a new file and call fchmod
(which I am proposing we bind) or chmod if that is not available.


> > I think this is wrong, and we should only chmod when a mode is specified
> > AND:
> > the umask will result in a different mode on the output file if we
> > supply it ourselves
> > AND
> > we are making a new file not altering an existing one.
> > 
> > What do you think?
> > 
> 
> I would be happy with that. Problem #1 is that I don't know of any way
> to determine the current umask without setting it. (os.umask() will
> return the old umask, but only after you set a new one).

mask = os.umask(0)
os.umask(mask)

> I suppose at some startup time the LocalTransport could detect the
> umask, and then reset it and save the value from then on. Detecting if
> it needs to do a chmod, or whether it can just set the mode bits at open
> time.
> 
> Back when I added chmod, it didn't effect performance very much (which
> is why I also had flags which would allow you to disable setting chmod).
> I had done benchmarks, and didn't see an effective difference. Now that
> we may have cleaned up some other stuff, we may want to re-evaluate it.

Its not a huge cost, but it is overhead we dont need, and I thats one
thing I've taken away from the hg meeting, is to much more closely
examine 'is this worth paying'.

> We should also keep in mind how we would want to tune
> SFTPTransport.put() and .append()

Yup. 

I'll update my patch to do what we've discussed, with the exception of
binding fchmod for now.

Rob
-- 
GPG key available at: <http://www.robertcollins.net/keys.txt>.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 191 bytes
Desc: This is a digitally signed message part
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20060611/1e33f735/attachment.pgp