| Submitter | Gregory Szorc |
|---|---|
| Date | Jan. 3, 2016, 12:45 a.m. |
| Message ID | <3523527da243b2d150d5.1451781933@ubuntu-vm-main> |
| Download | mbox | patch |
| Permalink | /patch/12499/ |
| State | Accepted |
| Delegated to: | Matt Mackall |
| Headers | show |
Comments
On Sat, Jan 02, 2016 at 04:45:33PM -0800, Gregory Szorc wrote: > # HG changeset patch > # User Gregory Szorc <gregory.szorc@gmail.com> > # Date 1451776198 28800 > # Sat Jan 02 15:09:58 2016 -0800 > # Node ID 3523527da243b2d150d54d77aa630b23b2e9ad7d > # Parent 438dce208879a651879d123d646c6782e3936f42 > streamclone: use context manager for writing files I'm a huge fan of this series. Are we okay with starting to use context managers for stuff like this? It sounds like a nice win for the abstraction needed for Greg's background IO business. > > These are the file writes that have the most to gain from background > I/O. Plug in a context manager so I can design the background I/O > mechanism with context managers in mind. > > diff --git a/mercurial/streamclone.py b/mercurial/streamclone.py > --- a/mercurial/streamclone.py > +++ b/mercurial/streamclone.py > @@ -314,22 +314,22 @@ def consumev1(repo, fp, filecount, bytec > size = int(size) > except (ValueError, TypeError): > raise error.ResponseError( > _('unexpected response from remote server:'), l) > if repo.ui.debugflag: > repo.ui.debug('adding %s (%s)\n' % > (name, util.bytecount(size))) > # for backwards compat, name was partially encoded > - ofp = repo.svfs(store.decodedir(name), 'w') > - for chunk in util.filechunkiter(fp, limit=size): > - handled_bytes += len(chunk) > - repo.ui.progress(_('clone'), handled_bytes, total=bytecount) > - ofp.write(chunk) > - ofp.close() > + with repo.svfs(store.decodedir(name), 'w') as ofp: > + for chunk in util.filechunkiter(fp, limit=size): > + handled_bytes += len(chunk) > + repo.ui.progress(_('clone'), handled_bytes, > + total=bytecount) > + ofp.write(chunk) > tr.close() > finally: > tr.release() > > # Writing straight to files circumvented the inmemory caches > repo.invalidate() > > elapsed = time.time() - start > _______________________________________________ > Mercurial-devel mailing list > Mercurial-devel@selenic.com > https://selenic.com/mailman/listinfo/mercurial-devel
On Tue, 2016-01-05 at 12:09 -0500, Augie Fackler wrote: > On Sat, Jan 02, 2016 at 04:45:33PM -0800, Gregory Szorc wrote: > > # HG changeset patch > > # User Gregory Szorc <gregory.szorc@gmail.com> > > # Date 1451776198 28800 > > # Sat Jan 02 15:09:58 2016 -0800 > > # Node ID 3523527da243b2d150d54d77aa630b23b2e9ad7d > > # Parent 438dce208879a651879d123d646c6782e3936f42 > > streamclone: use context manager for writing files > > I'm a huge fan of this series. Are we okay with starting to use > context managers for stuff like this? Yes. -- Mathematics is the supreme nostalgia of our time.
On Sat, Jan 02, 2016 at 04:45:33PM -0800, Gregory Szorc wrote: > # HG changeset patch > # User Gregory Szorc <gregory.szorc@gmail.com> > # Date 1451776198 28800 > # Sat Jan 02 15:09:58 2016 -0800 > # Node ID 3523527da243b2d150d54d77aa630b23b2e9ad7d > # Parent 438dce208879a651879d123d646c6782e3936f42 > streamclone: use context manager for writing files Queued these. I like how this one in particular avoids potential bugs (it wasn't even a try/finally before.) > > These are the file writes that have the most to gain from background > I/O. Plug in a context manager so I can design the background I/O > mechanism with context managers in mind. > > diff --git a/mercurial/streamclone.py b/mercurial/streamclone.py > --- a/mercurial/streamclone.py > +++ b/mercurial/streamclone.py > @@ -314,22 +314,22 @@ def consumev1(repo, fp, filecount, bytec > size = int(size) > except (ValueError, TypeError): > raise error.ResponseError( > _('unexpected response from remote server:'), l) > if repo.ui.debugflag: > repo.ui.debug('adding %s (%s)\n' % > (name, util.bytecount(size))) > # for backwards compat, name was partially encoded > - ofp = repo.svfs(store.decodedir(name), 'w') > - for chunk in util.filechunkiter(fp, limit=size): > - handled_bytes += len(chunk) > - repo.ui.progress(_('clone'), handled_bytes, total=bytecount) > - ofp.write(chunk) > - ofp.close() > + with repo.svfs(store.decodedir(name), 'w') as ofp: > + for chunk in util.filechunkiter(fp, limit=size): > + handled_bytes += len(chunk) > + repo.ui.progress(_('clone'), handled_bytes, > + total=bytecount) > + ofp.write(chunk) > tr.close() > finally: > tr.release() > > # Writing straight to files circumvented the inmemory caches > repo.invalidate() > > elapsed = time.time() - start > _______________________________________________ > Mercurial-devel mailing list > Mercurial-devel@selenic.com > https://selenic.com/mailman/listinfo/mercurial-devel
Patch
diff --git a/mercurial/streamclone.py b/mercurial/streamclone.py --- a/mercurial/streamclone.py +++ b/mercurial/streamclone.py @@ -314,22 +314,22 @@ def consumev1(repo, fp, filecount, bytec size = int(size) except (ValueError, TypeError): raise error.ResponseError( _('unexpected response from remote server:'), l) if repo.ui.debugflag: repo.ui.debug('adding %s (%s)\n' % (name, util.bytecount(size))) # for backwards compat, name was partially encoded - ofp = repo.svfs(store.decodedir(name), 'w') - for chunk in util.filechunkiter(fp, limit=size): - handled_bytes += len(chunk) - repo.ui.progress(_('clone'), handled_bytes, total=bytecount) - ofp.write(chunk) - ofp.close() + with repo.svfs(store.decodedir(name), 'w') as ofp: + for chunk in util.filechunkiter(fp, limit=size): + handled_bytes += len(chunk) + repo.ui.progress(_('clone'), handled_bytes, + total=bytecount) + ofp.write(chunk) tr.close() finally: tr.release() # Writing straight to files circumvented the inmemory caches repo.invalidate() elapsed = time.time() - start