This is an archive of the discontinued Mercurial Phabricator instance.

Differential D10469

profiling: add --no-profile to disable profiling enabled via config
ClosedPublic

Authored by spectral on Apr 19 2021, 6:32 PM.

Details

Reviewers
pulkit
Group Reviewers
hg-reviewers
Commits
rHGa2bf93ab3014: profiling: add --no-profile to disable profiling enabled via config

Diff Detail

Repository
rHG Mercurial
Branch
default
Lint
No Linters Available
Unit
No Unit Test Coverage

Event Timeline

spectral created this revision.Apr 19 2021, 6:32 PM
marmoute added a subscriber: marmoute.Apr 20 2021, 6:07 AM
marmoute added inline comments.
mercurial/dispatch.py
1066

Identify testing on boolean is not recommended. This should be elif req.earlyoptions[b'profile']

spectral added inline comments.Apr 20 2021, 1:22 PM
mercurial/dispatch.py
1066

If the user didn't specify --profile or --no-profile, then it's None here. Testing for an explicit False is intentional.

spectral marked an inline comment as done.Apr 21 2021, 4:18 PM

(marking comment as resolved, not sure what the exact policy is here :))

pulkit accepted this revision.Apr 28 2021, 6:39 AM
This revision is now accepted and ready to land.Apr 28 2021, 6:39 AM

Revision Contents
Changeset List

PathPackages
Mmercurial/dispatch.py (10 lines)
Mtests/test-chg.t (69 lines)

Diff 27058

mercurial/dispatch.py

for opt in (b'verbose', b'debug', b'quiet'): for opt in (b'verbose', b'debug', b'quiet'):
val = pycompat.bytestr(bool(req.earlyoptions[opt])) val = pycompat.bytestr(bool(req.earlyoptions[opt]))
for ui_ in uis: for ui_ in uis:
ui_.setconfig(b'ui', opt, val, b'--' + opt) ui_.setconfig(b'ui', opt, val, b'--' + opt)
if req.earlyoptions[b'profile']: if req.earlyoptions[b'profile']:
for ui_ in uis: for ui_ in uis:
ui_.setconfig(b'profiling', b'enabled', b'true', b'--profile') ui_.setconfig(b'profiling', b'enabled', b'true', b'--profile')
elif req.earlyoptions[b'profile'] is False:
marmouteUnsubmitted
Done

Identify testing on boolean is not recommended. This should be elif req.earlyoptions[b'profile']

marmoute: Identify testing on boolean is not recommended. This should be `elif req.earlyoptions…
spectralAuthorUnsubmitted
Done

If the user didn't specify --profile or --no-profile, then it's None here. Testing for an explicit False is intentional.

spectral: If the user didn't specify --profile or --no-profile, then it's None here. Testing for an…
# Check for it being set already, so that we don't pollute the config
# with this when using chg in the very common case that it's not
# enabled.
if lui.configbool(b'profiling', b'enabled'):
# Only do this on lui so that `chg foo` with a user config setting
# profiling.enabled=1 still shows profiling information (chg will
# specify `--no-profile` when `hg serve` is starting up, we don't
# want that to propagate to every later invocation).
lui.setconfig(b'profiling', b'enabled', b'false', b'--no-profile')
profile = lui.configbool(b'profiling', b'enabled') profile = lui.configbool(b'profiling', b'enabled')
with profiling.profile(lui, enabled=profile) as profiler: with profiling.profile(lui, enabled=profile) as profiler:
# Configure extensions in phases: uisetup, extsetup, cmdtable, and # Configure extensions in phases: uisetup, extsetup, cmdtable, and
# reposetup # reposetup
extensions.loadall(lui) extensions.loadall(lui)
# Propagate any changes to lui.__class__ by extensions # Propagate any changes to lui.__class__ by extensions
ui.__class__ = lui.__class__ ui.__class__ = lui.__class__

tests/test-chg.t

$ (unset LC_ALL; unset LANG; LC_CTYPE= chg \ $ (unset LC_ALL; unset LANG; LC_CTYPE= chg \
> --config extensions.debugenv=$TESTTMP/debugenv.py debugenv) > --config extensions.debugenv=$TESTTMP/debugenv.py debugenv)
LC_CTYPE= LC_CTYPE=
$ LANG= LC_ALL= LC_CTYPE= chg \ $ LANG= LC_ALL= LC_CTYPE= chg \
> --config extensions.debugenv=$TESTTMP/debugenv.py debugenv > --config extensions.debugenv=$TESTTMP/debugenv.py debugenv
LC_ALL= LC_ALL=
LC_CTYPE= LC_CTYPE=
LANG= LANG=
Profiling isn't permanently enabled or carried over between chg invocations that
share the same server
$ cp $HGRCPATH.orig $HGRCPATH
$ hg init $TESTTMP/profiling
$ cd $TESTTMP/profiling
$ filteredchg() {
> CHGDEBUG=1 chg "$@" 2>&1 | egrep 'Sample count|start cmdserver' || true
> }
$ newchg() {
> chg --kill-chg-daemon
> filteredchg "$@" | egrep -v 'start cmdserver' || true
> }
(--profile isn't permanently on just because it was specified when chg was
started)
$ newchg log -r . --profile
Sample count: * (glob)
$ filteredchg log -r .
(enabling profiling via config works, even on the first chg command that starts
a cmdserver)
$ cat >> $HGRCPATH <<EOF
> [profiling]
> type=stat
> enabled=1
> EOF
$ newchg log -r .
Sample count: * (glob)
$ filteredchg log -r .
Sample count: * (glob)
(test that we aren't accumulating more and more samples each run)
$ cat > $TESTTMP/debugsleep.py <<EOF
> import time
> from mercurial import registrar
> cmdtable = {}
> command = registrar.command(cmdtable)
> @command(b'debugsleep', [], b'', norepo=True)
> def debugsleep(ui):
> start = time.time()
> x = 0
> while time.time() < start + 0.5:
> time.sleep(.1)
> x += 1
> ui.status(b'%d debugsleep iterations in %.03fs\n' % (x, time.time() - start))
> EOF
$ cat >> $HGRCPATH <<EOF
> [extensions]
> debugsleep = $TESTTMP/debugsleep.py
> EOF
$ newchg debugsleep > run_1
$ filteredchg debugsleep > run_2
$ filteredchg debugsleep > run_3
$ filteredchg debugsleep > run_4
FIXME: Run 4 should not be >3x Run 1's number of samples.
$ "$PYTHON" <<EOF
> r1 = int(open("run_1", "r").read().split()[-1])
> r4 = int(open("run_4", "r").read().split()[-1])
> print("Run 1: %d samples\nRun 4: %d samples\nRun 4 > 3 * Run 1: %s" %
> (r1, r4, r4 > (r1 * 3)))
> EOF
Run 1: * samples (glob)
Run 4: * samples (glob)
Run 4 > 3 * Run 1: True
(Disabling with --no-profile on the commandline still works, but isn't permanent)
$ newchg log -r . --no-profile
$ filteredchg log -r .
Sample count: * (glob)
$ filteredchg log -r . --no-profile
$ filteredchg log -r .
Sample count: * (glob)