This is an archive of the discontinued Mercurial Phabricator instance.

Differential D1708

treemanifest: store ui once instead of using repo.ui every time
AbandonedPublic

Authored by singhsrb on Dec 15 2017, 3:33 PM.

Download Raw Diff

Details

Reviewers

quark

Group Reviewers

Restricted Project

Summary

setuptreestores caters to a lot of configurations through the ui
instance. It makes sense to store the ui instance once instead of referring
to it through repo.ui every time.

Test Plan

Ran all the tests.

Diff Detail

Repository

rFBHGX Facebook Mercurial Extensions

Lint

Lint Skipped

Unit

Unit Tests Skipped

Event Timeline

singhsrb created this revision.Dec 15 2017, 3:33 PM

Herald added a reviewer: Restricted Project. · View Herald TranscriptDec 15 2017, 3:33 PM

I think the reason there are both ui and repo in @command interface is because some commands do not need a repo but need a ui. Or sometimes repo.ui is different from ui (in chg's case). In this case repo.ui is ui. It is cleaner to avoid two arguments.

treemanifest/__init__.py
281	Why not insert `ui = repo.ui` here?

singhsrb added inline comments.Dec 15 2017, 7:17 PM

treemanifest/__init__.py
281	Sure! that works as well! I wanted to be sure I am not missing something.

singhsrb edited the summary of this revision. (Show Details)Dec 15 2017, 7:24 PM

singhsrb retitled this revision from treemanifest: include ui in the method signature instead of using repo.ui to treemanifest: store ui once instead of using repo.ui every time.

singhsrb updated this revision to Diff 4522.

quark accepted this revision.Dec 15 2017, 7:28 PM

This revision is now accepted and ready to land.Dec 15 2017, 7:28 PM

singhsrb added a child revision: D1826: treemanifest: demand convert flat manifest to tree manifest in treeonly mode.Jan 5 2018, 11:51 PM

Will be rebasing and floating for review again.

Revision Contents
Changeset List

			Path	Packages
M			treemanifest/__init__.py (30 lines)

Status	Author	Revision
Abandoned	singhsrb	D1826 treemanifest: demand convert flat manifest to tree manifest in treeonly mode
Abandoned	singhsrb	D1825 treemanifest: add client support for flat manifest to tree manifest
Abandoned	singhsrb	D1824 treemanifest: refactor write manifest wrapper
Abandoned	singhsrb	D1823 backfill: refactor to segregate logic for converting manifests
Abandoned	singhsrb	D1822 test-treemanifest-treeonly: update test to have more flat manifests
Abandoned	singhsrb	D1709 remotetreedatastore: make MissingNodesError subclass of KeyError
Abandoned	singhsrb	D1708 treemanifest: store ui once instead of using repo.ui every time
Abandoned	singhsrb	D1819 treemanifest: write client tree manifests eagerly during nested transactions
Abandoned	singhsrb	D1818 treemanifest: use flat manifest hash during client tree manifest writes
Abandoned	singhsrb	D1817 treemanifest: separate out logic to write client tree manifest

Diff 4517

treemanifest/init.py

	config = repo.ui.configint("packs", "maxpackfilecount")			config = repo.ui.configint("packs", "maxpackfilecount")
	if config and numentries > config:			if config and numentries > config:
	repo.ui.warn(("purging shared treemanifest pack cache (%d entries) "			repo.ui.warn(("purging shared treemanifest pack cache (%d entries) "
	"-- too many files\n" % numentries))			"-- too many files\n" % numentries))
	shutil.rmtree(packpath, True)			shutil.rmtree(packpath, True)
	except OSError:			except OSError:
	pass			pass

	def setuptreestores(repo, mfl):			def setuptreestores(ui, repo, mfl):
	if repo.ui.configbool("treemanifest", "server"):			if ui.configbool('treemanifest', 'server'):
				quarkUnsubmitted Not Done Why not insert `ui = repo.ui` here? quark: Why not insert `ui = repo.ui` here?
				singhsrbAuthorUnsubmitted Not Done Sure! that works as well! I wanted to be sure I am not missing something. singhsrb: Sure! that works as well! I wanted to be sure I am not missing something.
	packpath = repo.vfs.join('cache/packs/%s' % PACK_CATEGORY)			packpath = repo.vfs.join('cache/packs/%s' % PACK_CATEGORY)

	# Data store			# Data store
	datastore = cstore.datapackstore(packpath)			datastore = cstore.datapackstore(packpath)
	revlogstore = manifestrevlogstore(repo)			revlogstore = manifestrevlogstore(repo)
	mfl.datastore = unioncontentstore(datastore, revlogstore)			mfl.datastore = unioncontentstore(datastore, revlogstore)

	# History store			# History store
	historystore = historypackstore(repo.ui, packpath)			historystore = historypackstore(ui, packpath)
	mfl.historystore = unionmetadatastore(			mfl.historystore = unionmetadatastore(
	historystore,			historystore,
	revlogstore,			revlogstore,
	)			)
	_prunesharedpacks(repo, packpath)			_prunesharedpacks(repo, packpath)
	return			return

	usecdatapack = repo.ui.configbool('remotefilelog', 'fastdatapack')			usecdatapack = ui.configbool('remotefilelog', 'fastdatapack')

	if not util.safehasattr(repo, 'name'):			if not util.safehasattr(repo, 'name'):
	repo.name = repo.ui.config('remotefilelog', 'reponame')			repo.name = ui.config('remotefilelog', 'reponame')
	packpath = shallowutil.getcachepackpath(repo, PACK_CATEGORY)			packpath = shallowutil.getcachepackpath(repo, PACK_CATEGORY)
	_prunesharedpacks(repo, packpath)			_prunesharedpacks(repo, packpath)

	localpackpath = shallowutil.getlocalpackpath(repo.svfs.vfs.base,			localpackpath = shallowutil.getlocalpackpath(repo.svfs.vfs.base,
	PACK_CATEGORY)			PACK_CATEGORY)

	# Data store			# Data store
	if repo.ui.configbool("treemanifest", "usecunionstore"):			if ui.configbool('treemanifest', 'usecunionstore'):
	datastore = cstore.datapackstore(packpath)			datastore = cstore.datapackstore(packpath)
	localdatastore = cstore.datapackstore(localpackpath)			localdatastore = cstore.datapackstore(localpackpath)
	# TODO: can't use remotedatastore with cunionstore yet			# TODO: can't use remotedatastore with cunionstore yet
	# TODO make reportmetrics work with cstore			# TODO make reportmetrics work with cstore
	mfl.datastore = cstore.uniondatapackstore([localdatastore, datastore])			mfl.datastore = cstore.uniondatapackstore([localdatastore, datastore])
	else:			else:
	datastore = datapackstore(repo.ui, packpath, usecdatapack=usecdatapack)			datastore = datapackstore(ui, packpath, usecdatapack=usecdatapack)
	localdatastore = datapackstore(repo.ui, localpackpath,			localdatastore = datapackstore(ui, localpackpath,
	usecdatapack=usecdatapack)			usecdatapack=usecdatapack)
	stores = [datastore, localdatastore]			stores = [datastore, localdatastore]
	remotedatastore = remotetreedatastore(repo)			remotedatastore = remotetreedatastore(repo)
	if repo.ui.configbool("treemanifest", "demanddownload", True):			if ui.configbool('treemanifest', 'demanddownload', True):
	stores.append(remotedatastore)			stores.append(remotedatastore)

	mfl.datastore = unioncontentstore(*stores,			mfl.datastore = unioncontentstore(*stores,
	writestore=localdatastore)			writestore=localdatastore)
	remotedatastore.setshared(mfl.datastore)			remotedatastore.setshared(mfl.datastore)

	mfl.shareddatastores = [datastore]			mfl.shareddatastores = [datastore]
	mfl.localdatastores = [localdatastore]			mfl.localdatastores = [localdatastore]
	mfl.ui = repo.ui			mfl.ui = ui

	# History store			# History store
	sharedhistorystore = historypackstore(repo.ui, packpath)			sharedhistorystore = historypackstore(ui, packpath)
	localhistorystore = historypackstore(repo.ui, localpackpath)			localhistorystore = historypackstore(ui, localpackpath)
	mfl.sharedhistorystores = [			mfl.sharedhistorystores = [
	sharedhistorystore			sharedhistorystore
	]			]
	mfl.localhistorystores = [			mfl.localhistorystores = [
	localhistorystore,			localhistorystore,
	]			]
	mfl.historystore = unionmetadatastore(			mfl.historystore = unionmetadatastore(
	sharedhistorystore,			sharedhistorystore,
	localhistorystore,			localhistorystore,
	writestore=localhistorystore,			writestore=localhistorystore,
	)			)
	shallowutil.reportpackmetrics(repo.ui, 'treestore', mfl.datastore,			shallowutil.reportpackmetrics(ui, 'treestore', mfl.datastore,
	mfl.historystore)			mfl.historystore)

	class treemanifestlog(manifest.manifestlog):			class treemanifestlog(manifest.manifestlog):
	def __init__(self, opener, treemanifest=False):			def __init__(self, opener, treemanifest=False):
	assert treemanifest is False			assert treemanifest is False
	cachesize = 4			cachesize = 4

	opts = getattr(opener, 'options', None)			opts = getattr(opener, 'options', None)
	"server without pushrebase"))			"server without pushrebase"))

	def getmanifestlog(orig, self):			def getmanifestlog(orig, self):
	if not treeenabled(self.ui):			if not treeenabled(self.ui):
	return orig(self)			return orig(self)

	if self.ui.configbool('treemanifest', 'treeonly'):			if self.ui.configbool('treemanifest', 'treeonly'):
	mfl = treeonlymanifestlog(self.svfs)			mfl = treeonlymanifestlog(self.svfs)
	setuptreestores(self, mfl)			setuptreestores(self.ui, self, mfl)
	else:			else:
	mfl = orig(self)			mfl = orig(self)
	mfl.treemanifestlog = treemanifestlog(self.svfs)			mfl.treemanifestlog = treemanifestlog(self.svfs)
	setuptreestores(self, mfl.treemanifestlog)			setuptreestores(self.ui, self, mfl.treemanifestlog)
	mfl.datastore = mfl.treemanifestlog.datastore			mfl.datastore = mfl.treemanifestlog.datastore
	mfl.historystore = mfl.treemanifestlog.historystore			mfl.historystore = mfl.treemanifestlog.historystore

	if util.safehasattr(mfl.treemanifestlog, 'shareddatastores'):			if util.safehasattr(mfl.treemanifestlog, 'shareddatastores'):
	mfl.shareddatastores = mfl.treemanifestlog.shareddatastores			mfl.shareddatastores = mfl.treemanifestlog.shareddatastores
	mfl.localdatastores = mfl.treemanifestlog.localdatastores			mfl.localdatastores = mfl.treemanifestlog.localdatastores
	mfl.sharedhistorystores = mfl.treemanifestlog.sharedhistorystores			mfl.sharedhistorystores = mfl.treemanifestlog.sharedhistorystores
	mfl.localhistorystores = mfl.treemanifestlog.localhistorystores			mfl.localhistorystores = mfl.treemanifestlog.localhistorystores

Diff	ID	Description	Created	Lint	Unit
Base		Base
Diff 1	4517		Dec 15 2017, 3:33 PM	★	★
Diff 2	4522		Dec 15 2017, 7:24 PM	★	★