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
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

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
1067

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
1067

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 27252

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)