This is an archive of the discontinued Mercurial Phabricator instance.

Differential D1217

basepack: make base store's refresh() oblivious to most recent packs
AbandonedPublic

Authored by singhsrb on Oct 23 2017, 4:07 PM.
Tags
None
Subscribers
None

Details

Reviewers
None
Group Reviewers
Restricted Project
Summary

D1208 adds a cache which anyways takes care of accessing the most
recent packs. Therefore, it is redundant for the refresh logic to care for the
most recent packs. Furthermore, it also simplifies the refresh code.

Test Plan
  • Ran all the tests.
  • Re-ran the testing with the large repo as in D1208 and verified that the performance was almost similar.

Diff Detail

Repository
rFBHGX Facebook Mercurial Extensions
Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

singhsrb created this revision.Oct 23 2017, 4:07 PM
Herald added a reviewer: Restricted Project. · View Herald TranscriptOct 23 2017, 4:07 PM
singhsrb planned changes to this revision.Oct 23 2017, 4:37 PM
singhsrb added inline comments.
remotefilelog/basepack.py
135

Probably can remove filenamewithoutext from fileswithoutext here.

singhsrb updated this revision to Diff 3071.Oct 23 2017, 4:43 PM
singhsrb abandoned this revision.Oct 23 2017, 11:43 PM

This won't be needed after suggested changes in D1208.

Revision Contents
Changeset List

PathPackages
Mremotefilelog/basepack.py (39 lines)

Diff 3071

remotefilelog/basepack.py

cachedpaths.add(path) cachedpaths.add(path)
yield path yield path
# yield the paths not in the cache. # yield the paths not in the cache.
for path in set(self._pathtopack.keys()) - cachedpaths: for path in set(self._pathtopack.keys()) - cachedpaths:
yield path yield path
def _getavailablepackfiles(self): def _getavailablepackfiles(self):
suffixlen = len(self.INDEXSUFFIX) indexsuffixlen = len(self.INDEXSUFFIX)
packsuffixlen = len(self.PACKSUFFIX)
files = [] fileswithoutext = set()
filenames = set()
try: try:
for filename, size, stat in osutil.listdir(self.path, stat=True): for filename, _size in osutil.listdir(self.path):
files.append((stat.st_mtime, filename)) filenamewithoutext = None
filenames.add(filename) if filename[-indexsuffixlen:] == self.INDEXSUFFIX:
filenamewithoutext = filename[:-indexsuffixlen]
elif filename[-packsuffixlen:] == self.PACKSUFFIX:
filenamewithoutext = filename[:-packsuffixlen]
# Since we expect to have two files corresponding to each
# 'filename without the extension' i.e. the index file and the
# pack file, we can yield the 'filename without the extension'
# if we see it more than one time.
if filenamewithoutext:
if filenamewithoutext in fileswithoutext:
fileswithoutext.remove(filenamewithoutext)
singhsrbAuthorUnsubmitted
Not Done

Probably can remove filenamewithoutext from fileswithoutext here.

singhsrb: Probably can remove filenamewithoutext from fileswithoutext here.
yield os.path.join(self.path, filenamewithoutext)
else:
fileswithoutext.add(filenamewithoutext)
except OSError as ex: except OSError as ex:
if ex.errno != errno.ENOENT: if ex.errno != errno.ENOENT:
raise raise
# TODO: Refactor/Remove this section because there are negligible
# performance benefits of fetching the most recent files here after
# adding the LRU cache.
# Put most recent pack files first since they contain the most recent
# info.
files = sorted(files, reverse=True)
for mtime, filename in files:
packfilename = '%s%s' % (filename[:-suffixlen], self.PACKSUFFIX)
if (filename[-suffixlen:] == self.INDEXSUFFIX
and packfilename in filenames):
yield os.path.join(self.path, filename)[:-suffixlen]
def getpack(self, path): def getpack(self, path):
raise NotImplemented() raise NotImplemented()
def getmissing(self, keys): def getmissing(self, keys):
missing = keys missing = keys
for path in self.pathtopack: for path in self.pathtopack:
pack = self.pathtopack[path] pack = self.pathtopack[path]