tests/test-treemanifest.t
71–74 ↗	(On Diff #3172)	hm, this is kinda interesting. I guess this triggered because `subdir/`'s entry interrupts this chain.

remotefilelog/debugcommands.py
216	I don't think this is a correct assumption? If you have a commit c1 with two parents, p1 and p2, it's possible that c1 could be delta'd against either p1 or p2, which may not be contiguous in the pack output.
217	We should explain why an entry is bad. Like: "BAD ENTRY - unexpected delta base"

remotefilelog/debugcommands.py
216	Ah, then it is incorrect. I'll rework it.

remotefilelog/debugcommands.py
212–213	Since we're sanity checking, should we warn about duplicate nodes too, while we're here?
293–295	It's probably not worth the optimization, but I think we could set `bases[node]` to `nullid` once we exit the while loop (complete the chain) to memoize and save us some iterations.

remotefilelog/debugcommands.py
278	If we're not using nodes as a list, I'd just make it a set from the beginning
293–295	I think we should actually. This algorithm is n^2, and if we have a delta chain that is 100,000 entries long then we're doing 10,000,000,000 unnecessary extra comparisons.

remotefilelog/debugcommands.py
293–295	sgtm

			Path	Packages
M			remotefilelog/debugcommands.py (49 lines)

Diff	ID	Description	Created	Lint	Unit
Base		Base
Diff 1	3169		Oct 31 2017, 12:32 AM	★	★
Diff 2	3172		Oct 31 2017, 2:02 AM	★	★
Diff 3	3293		Nov 6 2017, 12:55 PM	★	★
Diff 4	3309		Nov 7 2017, 11:58 AM	★	★
Diff 5	3310		Nov 7 2017, 12:00 PM	★	★
Diff 6	3318		Nov 7 2017, 1:56 PM	★	★
Diff 7	3319		Nov 7 2017, 2:02 PM	★	★
Diff 8	3320		Nov 7 2017, 2:05 PM	★	★

Status	Author	Revision
Closed	phillco	D1272 repack: sort orphan nodes by size
Closed	phillco	D1271 debugdatapack: return 1 and warn if there are invalid entries
Closed	phillco	D1259 debugdatapack: print delta and blob size totals and compression %
Closed	phillco	D1255 repack: add blob size to metadata if it's missing
Closed	phillco	D1254 debugdatapack: add blob size

Diff 3320

remotefilelog/debugcommands.py

	ui.write("%s:\n" % path)			ui.write("%s:\n" % path)
	dpack = datapack.datapack(path)			dpack = datapack.datapack(path)
	node = opts.get('node')			node = opts.get('node')
	if node:			if node:
	deltachain = dpack.getdeltachain('', bin(node))			deltachain = dpack.getdeltachain('', bin(node))
	dumpdeltachain(ui, deltachain, **opts)			dumpdeltachain(ui, deltachain, **opts)
	return			return

	if opts.get('long'):			if opts.get('long'):
	hashformatter = hex			hashformatter = hex
				phillcoAuthorUnsubmitted Not Done Since we're sanity checking, should we warn about duplicate nodes too, while we're here? phillco: Since we're sanity checking, should we warn about duplicate nodes too, while we're here?
	hashlen = 42			hashlen = 42
	else:			else:
	hashformatter = short			hashformatter = short
				durhamUnsubmitted Done I don't think this is a correct assumption? If you have a commit c1 with two parents, p1 and p2, it's possible that c1 could be delta'd against either p1 or p2, which may not be contiguous in the pack output. durham: I don't think this is a correct assumption? If you have a commit c1 with two parents, p1 and…
				phillcoAuthorUnsubmitted Done Ah, then it is incorrect. I'll rework it. phillco: Ah, then it is incorrect. I'll rework it.
	hashlen = 14			hashlen = 14
				durhamUnsubmitted Done We should explain why an entry is bad. Like: "BAD ENTRY - unexpected delta base" durham: We should explain why an entry is bad. Like: "BAD ENTRY - unexpected delta base"

	lastfilename = None			lastfilename = None
	totaldeltasize = 0			totaldeltasize = 0
	totalblobsize = 0			totalblobsize = 0
	def printtotals():			def printtotals():
	if lastfilename is not None:			if lastfilename is not None:
	ui.write("\n")			ui.write("\n")
	if not totaldeltasize or not totalblobsize:			if not totaldeltasize or not totalblobsize:
	return			return
	difference = totalblobsize - totaldeltasize			difference = totalblobsize - totaldeltasize
	deltastr = "%0.1f%% %s" % (			deltastr = "%0.1f%% %s" % (
	(100.0 * abs(difference) / totalblobsize),			(100.0 * abs(difference) / totalblobsize),
	("smaller" if difference > 0 else "bigger"))			("smaller" if difference > 0 else "bigger"))

	ui.write(("Total:%s%s %s (%s)\n") % (			ui.write(("Total:%s%s %s (%s)\n") % (
	"".ljust(2 * hashlen - len("Total:")),			"".ljust(2 * hashlen - len("Total:")),
	str(totaldeltasize).ljust(12),			str(totaldeltasize).ljust(12),
	str(totalblobsize).ljust(9),			str(totalblobsize).ljust(9),
	deltastr			deltastr
	))			))

				bases = {}
				nodes = set()
				failures = 0
	for filename, node, deltabase, deltalen in dpack.iterentries():			for filename, node, deltabase, deltalen in dpack.iterentries():
				bases[node] = deltabase
				if node in nodes:
				ui.write(("Bad entry: %s appears twice\n" % short(node)))
				failures += 1
				nodes.add(node)
	if filename != lastfilename:			if filename != lastfilename:
	printtotals()			printtotals()
	name = '(empty name)' if filename == '' else filename			name = '(empty name)' if filename == '' else filename
	ui.write("%s:\n" % name)			ui.write("%s:\n" % name)
	ui.write("%s%s%s%s\n" % (			ui.write("%s%s%s%s\n" % (
	"Node".ljust(hashlen),			"Node".ljust(hashlen),
	"Delta Base".ljust(hashlen),			"Delta Base".ljust(hashlen),
	"Delta Length".ljust(14),			"Delta Length".ljust(14),
	"Blob Size".ljust(9)))			"Blob Size".ljust(9)))
	lastfilename = filename			lastfilename = filename
	totalblobsize = 0			totalblobsize = 0
	totaldeltasize = 0			totaldeltasize = 0

	# Metadata could be missing, in which case it will be an empty dict.			# Metadata could be missing, in which case it will be an empty dict.
	meta = dpack.getmeta(filename, node)			meta = dpack.getmeta(filename, node)
	if constants.METAKEYSIZE in meta:			if constants.METAKEYSIZE in meta:
	blobsize = meta[constants.METAKEYSIZE]			blobsize = meta[constants.METAKEYSIZE]
	totaldeltasize += deltalen			totaldeltasize += deltalen
	totalblobsize += blobsize			totalblobsize += blobsize
	else:			else:
	blobsize = "(missing)"			blobsize = "(missing)"
	ui.write("%s %s %s%s\n" % (			ui.write("%s %s %s%s\n" % (
	hashformatter(node),			hashformatter(node),
	hashformatter(deltabase),			hashformatter(deltabase),
	str(deltalen).ljust(14),			str(deltalen).ljust(14),
	blobsize))			blobsize))

	if filename is not None:			if filename is not None:
	printtotals()			printtotals()

				failures += _sanitycheck(ui, set(nodes), bases)
				durhamUnsubmitted Not Done If we're not using nodes as a list, I'd just make it a set from the beginning durham: If we're not using nodes as a list, I'd just make it a set from the beginning
				if failures > 1:
				ui.warn(("%d failures\n" % failures))
				return 1

				def _sanitycheck(ui, nodes, bases):
				"""
				Does some basic sanity checking on a packfiles with ``nodes`` ``bases`` (a
				mapping of node->base):

				- Each deltabase must itself be a node elsewhere in the pack
				- There must be no cycles
				"""
				failures = 0
				for node in nodes:
				seen = set()
				current = node
				deltabase = bases[current]
				phillcoAuthorUnsubmitted Not Done It's probably not worth the optimization, but I think we could set `bases[node]` to `nullid` once we exit the while loop (complete the chain) to memoize and save us some iterations. phillco: It's probably not worth the optimization, but I think we could set `bases[node]` to `nullid`…
				durhamUnsubmitted Not Done I think we should actually. This algorithm is n^2, and if we have a delta chain that is 100,000 entries long then we're doing 10,000,000,000 unnecessary extra comparisons. durham: I think we should actually. This algorithm is n^2, and if we have a delta chain that is 100…
				phillcoAuthorUnsubmitted Not Done sgtm phillco: sgtm

				while deltabase != nullid:
				if deltabase not in nodes:
				ui.warn(("Bad entry: %s has an unknown deltabase (%s)\n" %
				(short(node), short(deltabase))))
				failures += 1
				break

				if deltabase in seen:
				ui.warn(("Bad entry: %s has a cycle (at %s)\n" %
				(short(node), short(deltabase))))
				failures += 1
				break

				current = deltabase
				seen.add(current)
				deltabase = bases[current]
				# Since ``node`` begins a valid chain, reset/memoize its base to nullid
				# so we don't traverse it again.
				bases[node] = nullid
				return failures

	def dumpdeltachain(ui, deltachain, **opts):			def dumpdeltachain(ui, deltachain, **opts):
	hashformatter = hex			hashformatter = hex
	hashlen = 40			hashlen = 40

	lastfilename = None			lastfilename = None
	for filename, node, filename, deltabasenode, delta in deltachain:			for filename, node, filename, deltabasenode, delta in deltachain:
	if filename != lastfilename:			if filename != lastfilename:
	ui.write("\n%s\n" % filename)			ui.write("\n%s\n" % filename)

This is an archive of the discontinued Mercurial Phabricator instance.

debugdatapack: return 1 and warn if there are invalid entries
ClosedPublic

Details

Diff Detail

Event Timeline

Revision Contents
Changeset List

Diff 3320

remotefilelog/debugcommands.py

This is an archive of the discontinued Mercurial Phabricator instance.

debugdatapack: return 1 and warn if there are invalid entriesClosedPublic

Details

Diff Detail

Event Timeline

Revision ContentsChangeset List

Diff 3320

remotefilelog/debugcommands.py

debugdatapack: return 1 and warn if there are invalid entries
ClosedPublic

Revision Contents
Changeset List