D5296: store: don't read the whole fncache in memory

pulkit (Pulkit Goyal) phabricator at mercurial-scm.org
Sat Mar 16 20:37:13 EDT 2019


pulkit added a comment.


  In https://phab.mercurial-scm.org/D5296#88016, @yuja wrote:
  
  > > - self.entries = set(decodedir(fp.read()).splitlines()) + +        self.entries = [] +        totalsize = self.vfs.stat('fncache').st_size
  >
  > I don't think `totalsize` has to be stat()ed. We can just loop over until
  >  `fp.read()` reaches to EOF. It's unreliable to assume that the stat('fncache')
  >  see the identical file to the `fp` open()ed before.
  >
  > > +        chunksize = (10 ** 6) # 1 MB
  > >  +        chunk = b''
  > >  +        chunksize = min(totalsize, chunksize)
  >
  > Do we have any test covering `totalsize > chunksize` case?
  
  
  Right now this patch does not have any tests. How should I add them?
  
  1. add a debug config option and pass that to fncachestore and then to fncache
  2. have a function which returns the chunk_size, write an extenion in tests which wrap that function and enable that extension in tests
  
  I think 2) is better option here. I am unable to think of any solution expect this two. I think you will have better ideas here. What do you think?

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D5296

To: pulkit, #hg-reviewers
Cc: indygreg, yuja, mjpieters, mercurial-devel


More information about the Mercurial-devel mailing list