D6581: update: fix spurious unclean status bug shown by previous commit
Closed by commit rHGd29db0a0c4eb: update: fix spurious unclean status bug shown
by previous commit (authored by valentin.gatienbaron).
This revision was automatically updated to reflect the committed changes.
REPOSITORY
rHG Mercurial
CHANGES SINCE LAST UPDATE
https://phab.mercurial-scm.org/D6581?vs=15669&id=15674
CHANGES SINCE LAST ACTION
https://phab.mercurial-scm.org/D6581/new/
REVISION DETAIL
https://phab.mercurial-scm.org/D6581
AFFECTED FILES
mercurial/merge.py
mercurial/worker.py
tests/test-simple-update.t
CHANGE DETAILS
diff --git a/tests/test-simple-update.t b/tests/test-simple-update.t
--- a/tests/test-simple-update.t
+++ b/tests/test-simple-update.t
@@ -110,24 +110,6 @@
getting 100
100 files updated, 0 files merged, 0 files removed, 0 files unresolved
$ hg status
- M 100
- M 11
- M 2
- M 21
- M 3
- M 4
- M 41
- M 5
- M 51
- M 54
- M 6
- M 61
- M 7
- M 71
- M 8
- M 81
- M 9
- M 91
$ cd ..
diff --git a/mercurial/worker.py b/mercurial/worker.py
--- a/mercurial/worker.py
+++ b/mercurial/worker.py
@@ -100,8 +100,9 @@
workers
hasretval - when True, func and the current function return an progress
-iterator then a list (encoded as an iterator that yield many (False, ..)
-then a (True, list)). The resulting list is in the natural order.
+iterator then a dict (encoded as an iterator that yield many (False, ..)
+then a (True, dict)). The dicts are joined in some arbitrary order, so
+overlapping keys are a bad idea.
threadsafe - whether work items are thread safe and can be executed using
a thread-based worker. Should be disabled for CPU heavy tasks that don't
@@ -162,8 +163,8 @@
ui.flush()
parentpid = os.getpid()
pipes = []
-retvals = []
-for i, pargs in enumerate(partition(args, workers)):
+retval = {}
+for pargs in partition(args, workers):
# Every worker gets its own pipe to send results on, so we don't have
to
# implement atomic writes larger than PIPE_BUF. Each forked process has
# its own pipe's descriptors in the local variables, and the parent
@@ -171,7 +172,6 @@
# care what order they're in).
rfd, wfd = os.pipe()
pipes.append((rfd, wfd))
-retvals.append(None)
# make sure we use os._exit in all worker code paths. otherwise the
# worker may do some clean-ups which could cause surprises like
# deadlock. see sshpeer.cleanup for example.
@@ -192,7 +192,7 @@
os.close(w)
os.close(rfd)
for result in func(*(staticargs + (pargs,))):
-os.write(wfd, util.pickle.dumps((i, result)))
+os.write(wfd, util.pickle.dumps(result))
return 0
ret = scmutil.callcatch(ui, workerfunc)
@@ -226,9 +226,9 @@
while openpipes > 0:
for key, events in selector.select():
try:
-i, res = util.pickle.load(key.fileobj)
+res = util.pickle.load(key.fileobj)
if hasretval and res[0]:
-retvals[i] = res[1]
+retval.update(res[1])
else:
yield res
except EOFError:
@@ -249,7 +249,7 @@
os.kill(os.getpid(), -status)
sys.exit(status)
if hasretval:
-yield True, sum(retvals, [])
+yield True, retval
def _posixexitstatus(code):
'''convert a posix exit status into the same form returned by
@@ -281,9 +281,9 @@
try:
while not self._taskqueue.empty():
try:
-i, args = self._taskqueue.get_nowait()
+args = self._taskqueue.get_nowait()
for res in self._func(*self._staticargs + (args,)):
-self._resultqueue.put((i, res))
+self._resultqueue.put(res)
# threading doesn't provide a native way to
# interrupt execution. handle it manually at every
# iteration.
@@ -318,11 +318,10 @@
workers = _numworkers(ui)
resultqueue = pycompat.queue.Queue()
taskqueue = pycompat.queue.Queue()
-retvals = []
+retval = {}
# partition work to more pieces than workers to minimize the chance
# of uneven distribution of large tasks between the workers
-for pargs in enumerate(partition(args, workers * 20)):
-retvals.append(None)
+for pargs in partition(args, workers * 20):
taskqueue.put(pargs)
for _i in range(workers):
t = Worker(taskqueue, resultqueue, func, staticargs)
@@ -331,9 +330,9 @@
try:
while len(threads) > 0:
while not resultqueue.empty():
-(i, res) = resultqueue.get()
+
D6581: update: fix spurious unclean status bug shown by previous commit
This revision is now accepted and ready to land. martinvonz added a comment. martinvonz accepted this revision. Thanks for fixing this quickly! The fix looks very straight-forward too. REPOSITORY rHG Mercurial CHANGES SINCE LAST ACTION https://phab.mercurial-scm.org/D6581/new/ REVISION DETAIL https://phab.mercurial-scm.org/D6581 To: valentin.gatienbaron, #hg-reviewers, martinvonz Cc: martinvonz, mercurial-devel ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D6581: update: fix spurious unclean status bug shown by previous commit
valentin.gatienbaron created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.
REVISION SUMMARY
The crux of the problem is:
- the dirstate is corrupted (the sizes/dates are assigned to the wrong files)
- because when worker.worker is used with a return value (batchget in
merge.py here), the return value when worker.worker effectively parallelizes is
permuted
- this is because worker.worker's partition of input and combination of
output values are not inverses of one another: it split [1,2,3,4,5,6] into
[[1,3,5],[2,4,6]], but combines that into [1,3,5,2,4,6].
Given that worker.worker doesn't call its function argument on contiguous
chunks on the input arguments, sticking with lists means we'd need to
know the relation between the inputs of worker.worker function argument
(for instance, requiring that every input element is mapped to exactly
one output element). It seems better to instead switch return values to
dicts, which can combined reliably with a straighforward restriction.
REPOSITORY
rHG Mercurial
REVISION DETAIL
https://phab.mercurial-scm.org/D6581
AFFECTED FILES
mercurial/merge.py
mercurial/worker.py
tests/test-simple-update.t
CHANGE DETAILS
diff --git a/tests/test-simple-update.t b/tests/test-simple-update.t
--- a/tests/test-simple-update.t
+++ b/tests/test-simple-update.t
@@ -110,24 +110,6 @@
getting 100
100 files updated, 0 files merged, 0 files removed, 0 files unresolved
$ hg status
- M 100
- M 11
- M 2
- M 21
- M 3
- M 4
- M 41
- M 5
- M 51
- M 54
- M 6
- M 61
- M 7
- M 71
- M 8
- M 81
- M 9
- M 91
$ cd ..
diff --git a/mercurial/worker.py b/mercurial/worker.py
--- a/mercurial/worker.py
+++ b/mercurial/worker.py
@@ -100,8 +100,9 @@
workers
hasretval - when True, func and the current function return an progress
-iterator then a list (encoded as an iterator that yield many (False, ..)
-then a (True, list)). The resulting list is in the natural order.
+iterator then a dict (encoded as an iterator that yield many (False, ..)
+then a (True, dict)). The dicts are joined in some arbitrary order, so
+overlapping keys are a bad idea.
threadsafe - whether work items are thread safe and can be executed using
a thread-based worker. Should be disabled for CPU heavy tasks that don't
@@ -162,8 +163,8 @@
ui.flush()
parentpid = os.getpid()
pipes = []
-retvals = []
-for i, pargs in enumerate(partition(args, workers)):
+retval = {}
+for pargs in partition(args, workers):
# Every worker gets its own pipe to send results on, so we don't have
to
# implement atomic writes larger than PIPE_BUF. Each forked process has
# its own pipe's descriptors in the local variables, and the parent
@@ -171,7 +172,6 @@
# care what order they're in).
rfd, wfd = os.pipe()
pipes.append((rfd, wfd))
-retvals.append(None)
# make sure we use os._exit in all worker code paths. otherwise the
# worker may do some clean-ups which could cause surprises like
# deadlock. see sshpeer.cleanup for example.
@@ -192,7 +192,7 @@
os.close(w)
os.close(rfd)
for result in func(*(staticargs + (pargs,))):
-os.write(wfd, util.pickle.dumps((i, result)))
+os.write(wfd, util.pickle.dumps(result))
return 0
ret = scmutil.callcatch(ui, workerfunc)
@@ -226,9 +226,9 @@
while openpipes > 0:
for key, events in selector.select():
try:
-i, res = util.pickle.load(key.fileobj)
+res = util.pickle.load(key.fileobj)
if hasretval and res[0]:
-retvals[i] = res[1]
+retval.update(res[1])
else:
yield res
except EOFError:
@@ -249,7 +249,7 @@
os.kill(os.getpid(), -status)
sys.exit(status)
if hasretval:
-yield True, sum(retvals, [])
+yield True, retval
def _posixexitstatus(code):
'''convert a posix exit status into the same form returned by
@@ -281,9 +281,9 @@
try:
while not self._taskqueue.empty():
try:
-i, args = self._taskqueue.get_nowait()
+args = self._taskqueue.get_nowait()
for res in self._func(*self._staticargs + (args,)):
-self._resultqueue.put((i, res))
+self._resultqueue.put(res)
# threading doesn't provide a native way to
# interrupt execution. handle it manually at every
# iteration.
@@ -318,11 +318,10 @@
w
