This is an archive of the discontinued Mercurial Phabricator instance.

statprof: fix off-by-one-line error in output
ClosedPublic

Authored by spectral on Dec 2 2020, 10:20 PM.

Download Raw Diff

Details

Reviewers

martinvonz
pulkit

Group Reviewers

hg-reviewers

Commits

rHG068307b638f4: statprof: fix off-by-one-line error in output

Summary

martinvonz claims they thought that this was intentional, but couldn't remember
the reasoning for it. I can't understand why it would be preferable, and I
didn't see anything in the comments in the file about why this would be useful,
so I'm hopefully not breaking anything by "fixing" it.

Old output

| 100.0%  0.01s  dispatch.py:    run               line 43:  dispatch.run()
| 100.0%  0.01s  dispatch.py:    dispatch          line 115:  status = dispatch(req)
| 100.0%  0.01s  dispatch.py:    _runcatch         line 266:  ret = _runcatch(req) or 0
| 100.0%  0.01s  dispatch.py:    _callcatch        line 442:  return _callcatch(ui, _runc...
| 100.0%  0.01s  scmutil.py:     callcatch         line 451:  return scmutil.callcatch(ui...
| 100.0%  0.01s  dispatch.py:    _runcatchfunc     line 155:  return func()
| 100.0%  0.01s  dispatch.py:    _dispatch         line 432:  return _dispatch(req)
| 100.0%  0.01s  hg.py:          repository        line 1179:  repo = hg.repository(
| 100.0%  0.01s  hg.py:          _peerorrepo       line 221:  peer = _peerorrepo(
| 100.0%  0.01s  util.py:        __getattribute__  line 188:  obj = _peerlookup(path).ins...
| 100.0%  0.01s  localrepo.py:   makelocalrepositoryline 3227:  return makelocalrepository(...
| 100.0%  0.01s  localrepo.py:   __init__          line 683:  return cls(
| 100.0%  0.01s  util.py:        __getattribute__  line 1262:  self._extrafilterid = repov...
| 100.0%  0.01s  <frozen importlib._bootstrap_external>: exec_moduleline          245:  self.__spec__.loader.exec_m...
| 100.0%  0.01s  <frozen importlib._bootstrap_external>: get_codeline       779:
| 100.0%  0.01s  <frozen importlib._bootstrap_external>: path_statsline         868:
| 100.0%  0.01s  <frozen importlib._bootstrap_external>: _path_statline         1012:

New output

| 100.0%  0.01s  hg:             <module>          line 43:  dispatch.run()
| 100.0%  0.01s  dispatch.py:    run               line 115:  status = dispatch(req)
| 100.0%  0.01s  dispatch.py:    dispatch          line 266:  ret = _runcatch(req) or 0
| 100.0%  0.01s  dispatch.py:    _runcatch         line 442:  return _callcatch(ui, _runc...
| 100.0%  0.01s  dispatch.py:    _callcatch        line 451:  return scmutil.callcatch(ui...
| 100.0%  0.01s  scmutil.py:     callcatch         line 155:  return func()
| 100.0%  0.01s  dispatch.py:    _runcatchfunc     line 432:  return _dispatch(req)
| 100.0%  0.01s  dispatch.py:    _dispatch         line 1179:  repo = hg.repository(
| 100.0%  0.01s  hg.py:          repository        line 221:  peer = _peerorrepo(
| 100.0%  0.01s  hg.py:          _peerorrepo       line 188:  obj = _peerlookup(path).ins...
| 100.0%  0.01s  localrepo.py:   instance          line 3227:  return makelocalrepository(...
| 100.0%  0.01s  localrepo.py:   makelocalrepositoryline 683:  return cls(
| 100.0%  0.01s  localrepo.py:   __init__          line 1262:  self._extrafilterid = repov...
| 100.0%  0.01s  util.py:        __getattribute__  line 245:  self.__spec__.loader.exec_m...
| 100.0%  0.01s  <frozen importlib._bootstrap_external>: exec_moduleline          779:
| 100.0%  0.01s  <frozen importlib._bootstrap_external>: get_codeline       868:
| 100.0%  0.01s  <frozen importlib._bootstrap_external>: path_statsline         1012:
| 100.0%  0.01s  <frozen importlib._bootstrap_external>: _path_statline         87:

Diff Detail

Repository

rHG Mercurial

Branch

default

Lint

No Linters Available

Unit

No Unit Test Coverage

Event Timeline

spectral created this revision.Dec 2 2020, 10:20 PM

Herald added a reviewer: hg-reviewers. · View Herald TranscriptDec 2 2020, 10:20 PM

Herald added a subscriber: mercurial-patches. · View Herald Transcript

spectral added a child revision: D9511: statprof: separate functions and "line", assume 4 digit line numbers.Dec 2 2020, 10:20 PM

martinvonz claims they thought that this was intentional, but couldn't remember the reasoning for it.

I think I remember hearing durham@fb explain why it was that the it was but I'm not sure he argued that it should be the way it was. I agree that this seems like an obvious improvement :) Thanks!

This revision is now accepted and ready to land.Dec 3 2020, 12:37 AM

I have always found the old output confusing. Thank you for fixing it.

spectral added a commit: rHG068307b638f4: statprof: fix off-by-one-line error in output.Dec 3 2020, 4:38 AM

Closed by commit rHG068307b638f4: statprof: fix off-by-one-line error in output (authored by spectral). · Explain Why

This revision was automatically updated to reflect the committed changes.

baymax added a parent revision: D9513: phab-refresh: do not error out when the stack is empty.Dec 3 2020, 6:16 AM

baymax updated this revision to Diff 24002.

Revision Contents
Changeset List

			Path	Packages
M			mercurial/statprof.py (11 lines)
M			tests/test-profile.t (2 lines)

Diff	ID	Description	Created	Lint	Unit
Base		Base
Diff 1	23980		Dec 2 2020, 10:20 PM	★	★
Diff 2	23994	rHG068307b638f45e6bdab09b3030edb2cac2935b64	Dec 2 2020, 6:38 PM	★	★
Diff 3	24002		Dec 3 2020, 6:16 AM	★	★

Commit	Parents	Author	Summary	Date
97d68804342f	d42809b6b10f	Kyle Lippincott		Dec 2 2020, 6:38 PM

Status	Author	Revision
Closed	spectral	D9511 statprof: separate functions and "line", assume 4 digit line numbers
Closed	spectral	D9510 statprof: fix off-by-one-line error in output
Closed	marmoute	D9513 phab-refresh: do not error out when the stack is empty
Closed	marmoute	D9507 run-tests: allow some slack about 'waiting on lock' message
Closed	marmoute	D9506 pull: flush stdin after the `pull from` message
Closed	marmoute	D9505 tests: explicitly skip the lock warning in some remotefilelog tests
Closed	marmoute	D9504 tests: use the right python when running dummyssh for narrow
Closed	spectral	D9503 copies: avoid materializing a full directory map during copy tracing
Closed	martinvonz	D9509 rebase: clear merge state when aborting in-memory merge on dirty working copy
Closed	martinvonz	D9508 tests: show that in-memory rebase leaves state when working copy is dirty

Diff 23980

mercurial/statprof.py


	if len(stack) > 1:			if len(stack) > 1:
	i = 1			i = 1
	# Skip boiler plate parts of the stack			# Skip boiler plate parts of the stack
	while i < len(stack) and stack[i].skipname() in skips:			while i < len(stack) and stack[i].skipname() in skips:
	i += 1			i += 1
	if i < len(stack):			if i < len(stack):
	child.add(stack[i:], time)			child.add(stack[i:], time)
				else:
				# Normally this is done by the .add() calls
				child.count += time

	root = HotNode(None)			root = HotNode(None)
	lasttime = data.samples[0].time			lasttime = data.samples[0].time
	for sample in data.samples:			for sample in data.samples:
	root.add(sample.stack[::-1], sample.time - lasttime)			root.add(sample.stack[::-1], sample.time - lasttime)
	lasttime = sample.time			lasttime = sample.time
	showtime = kwargs.get('showtime', True)			showtime = kwargs.get('showtime', True)

	def _write(node, depth, multiple_siblings):			def _write(node, depth, multiple_siblings):
	site = node.site			site = node.site
	visiblechildren = [			visiblechildren = [
	c			c
	for c in pycompat.itervalues(node.children)			for c in pycompat.itervalues(node.children)
	if c.count >= (limit * root.count)			if c.count >= (limit * root.count)
	]			]
	if site:			if site:
	indent = depth * 2 - 1			indent = depth * 2 - 1
	filename = b''			filename = (site.filename() + b':').ljust(15)
	function = b''			function = site.function
	if len(node.children) > 0:
	childsite = list(pycompat.itervalues(node.children))[0].site
	filename = (childsite.filename() + b':').ljust(15)
	function = childsite.function

	# lots of string formatting			# lots of string formatting
	listpattern = (			listpattern = (
	b''.ljust(indent)			b''.ljust(indent)
	+ (b'\\' if multiple_siblings else b'\|')			+ (b'\\' if multiple_siblings else b'\|')
	+ b' %4.1f%%'			+ b' %4.1f%%'
	+ (b' %5.2fs' % node.count if showtime else b'')			+ (b' %5.2fs' % node.count if showtime else b'')
	+ b' %s %s'			+ b' %s %s'

tests/test-profile.t


	$ hg --profile --config profiling.statformat=bymethod sleep 2>../out \|\| cat ../out			$ hg --profile --config profiling.statformat=bymethod sleep 2>../out \|\| cat ../out
	$ head -n 1 ../out			$ head -n 1 ../out
	% cumulative self			% cumulative self
	$ cat ../out \| statprofran			$ cat ../out \| statprofran

	$ hg --profile --config profiling.statformat=hotpath sleep 2>../out \|\| cat ../out			$ hg --profile --config profiling.statformat=hotpath sleep 2>../out \|\| cat ../out
	$ cat ../out \| statprofran			$ cat ../out \| statprofran
				$ grep sleepext.py ../out
				.* [0-9.]+% [0-9.]+s sleepext.py:\ssleep line 7: time\.sleep. (re)

	$ hg --profile --config profiling.statformat=json sleep 2>../out \|\| cat ../out			$ hg --profile --config profiling.statformat=json sleep 2>../out \|\| cat ../out
	$ cat ../out			$ cat ../out
	\[\[-?\d+.* (re)			\[\[-?\d+.* (re)

	statprof can be used as a standalone module			statprof can be used as a standalone module

	$ "$PYTHON" -m mercurial.statprof hotpath			$ "$PYTHON" -m mercurial.statprof hotpath