This is an archive of the discontinued Mercurial Phabricator instance.

Differential D1460

workers: add config to enable/diable workers
ClosedPublic

Authored by wlis on Nov 20 2017, 1:37 PM.

Details

Reviewers
lothiraldan
ikostia
durin42
Group Reviewers
hg-reviewers
Commits
rHG471918fa7f46: workers: add config to enable/diable workers
rHG9b7ee7b78856: workers: add config to enable/diable workers
Summary

This adds config to disable/enable workers with default being enabled.

Test Plan

enabled profile without updaing .hg/hgrc (the default should be to use workers) and ran
hg sprase --enable-profile <profile>.sparse
Watched in the proces explorer that hg started 12 new threads for materializing files (this is my worker.numcpus) value

Added

[worker]
enabled = False

to the .hg/hgrc and re ran the command. This time hg didn't spawn any new threads for matreializing of files

Diff Detail

Repository
rHG Mercurial
Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

wlis created this revision.Nov 20 2017, 1:37 PM
wlis retitled this revision from workers: add config to enable/diable workers to workers: add config to enable/disable workers.Nov 20 2017, 1:44 PM
lothiraldan requested changes to this revision.Nov 22 2017, 3:28 AM
lothiraldan added a subscriber: lothiraldan.

Looks good.

We are in the process of registering all the config options in the file named configitems.py (https://www.mercurial-scm.org/repo/hg/file/tip/mercurial/configitems.py#l1132), could you update the file with the new option worker.enabled. By registering the option, we will be able to centralize default values (so you can even remove the default value in the configbol call) and do nice things like validating the user configuration or easily register aliases.

This revision now requires changes to proceed.Nov 22 2017, 3:28 AM
wlis retitled this revision from workers: add config to enable/disable workers to workers: add config to enable/diable workers.Nov 30 2017, 7:02 PM
wlis updated this revision to Diff 4023.
ikostia accepted this revision.Dec 1 2017, 11:12 AM
durin42 accepted this revision.Dec 5 2017, 5:27 PM
Closed by commit rHG9b7ee7b78856: workers: add config to enable/diable workers (authored by wlis). · Explain WhyDec 15 2017, 12:13 PM
This revision was automatically updated to reflect the committed changes.
indygreg added a subscriber: indygreg.Dec 17 2017, 2:30 PM
indygreg added inline comments.
mercurial/worker.py
85–86

I updated this line to drop the default value argument, which is not needed when the config item defines a default.

The test harness passed with the original code. The developer warning is emitted at run time. So this tells me that we have 0 test coverage of this workers code :/

Revision Contents
Changeset List

Diff 4023

mercurial/configitems.py

default=384, default=384,
) )
coreconfigitem('worker', 'backgroundcloseminfilecount', coreconfigitem('worker', 'backgroundcloseminfilecount',
default=2048, default=2048,
) )
coreconfigitem('worker', 'backgroundclosethreadcount', coreconfigitem('worker', 'backgroundclosethreadcount',
default=4, default=4,
) )
coreconfigitem('worker', 'enabled',
default=True,
)
coreconfigitem('worker', 'numcpus', coreconfigitem('worker', 'numcpus',
default=None, default=None,
) )
# Rebase related configuration moved to core because other extension are doing # Rebase related configuration moved to core because other extension are doing
# strange things. For example, shelve import the extensions to reuse some bit # strange things. For example, shelve import the extensions to reuse some bit
# without formally loading it. # without formally loading it.
coreconfigitem('commands', 'rebase.requiredest', coreconfigitem('commands', 'rebase.requiredest',
default=False, default=False,
) )
coreconfigitem('experimental', 'rebaseskipobsolete', coreconfigitem('experimental', 'rebaseskipobsolete',
default=True, default=True,
) )
coreconfigitem('rebase', 'singletransaction', coreconfigitem('rebase', 'singletransaction',
default=False, default=False,
) )

mercurial/help/config.txt

``worker`` ``worker``
---------- ----------
Parallel master/worker configuration. We currently perform working Parallel master/worker configuration. We currently perform working
directory updates in parallel on Unix-like systems, which greatly directory updates in parallel on Unix-like systems, which greatly
helps performance. helps performance.
``enabled``
Whether to enable workers code to be used.
(default: true)
``numcpus`` ``numcpus``
Number of CPUs to use for parallel operations. A zero or Number of CPUs to use for parallel operations. A zero or
negative value is treated as ``use the default``. negative value is treated as ``use the default``.
(default: 4 or the number of CPUs on the system, whichever is larger) (default: 4 or the number of CPUs on the system, whichever is larger)
``backgroundclose`` ``backgroundclose``
Whether to enable closing file handles on background threads during certain Whether to enable closing file handles on background threads during certain
operations. Some platforms aren't very efficient at closing file operations. Some platforms aren't very efficient at closing file

mercurial/worker.py

func - function to run func - function to run
staticargs - arguments to pass to every invocation of the function staticargs - arguments to pass to every invocation of the function
args - arguments to split into chunks, to pass to individual args - arguments to split into chunks, to pass to individual
workers workers
''' '''
if worthwhile(ui, costperarg, len(args)): enabled = ui.configbool('worker', 'enabled', True)
if enabled and worthwhile(ui, costperarg, len(args)):
indygregUnsubmitted
Not Done

I updated this line to drop the default value argument, which is not needed when the config item defines a default.

The test harness passed with the original code. The developer warning is emitted at run time. So this tells me that we have 0 test coverage of this workers code :/

indygreg: I updated this line to drop the default value argument, which is not needed when the config…
return _platformworker(ui, func, staticargs, args) return _platformworker(ui, func, staticargs, args)
return func(*staticargs + (args,)) return func(*staticargs + (args,))
def _posixworker(ui, func, staticargs, args): def _posixworker(ui, func, staticargs, args):
rfd, wfd = os.pipe() rfd, wfd = os.pipe()
workers = _numworkers(ui) workers = _numworkers(ui)
oldhandler = signal.getsignal(signal.SIGINT) oldhandler = signal.getsignal(signal.SIGINT)
signal.signal(signal.SIGINT, signal.SIG_IGN) signal.signal(signal.SIGINT, signal.SIG_IGN)