This is an archive of the discontinued Mercurial Phabricator instance.

Differential D9608

copies: stop attempt to avoid extra dict copies around branching
ClosedPublic

Authored by marmoute on Dec 15 2020, 4:07 AM.

Details

Reviewers
pulkit
Group Reviewers
hg-reviewers
Commits
rHGcb8b2ee89a5d: copies: stop attempt to avoid extra dict copies around branching
rHG39617270383f: copies: stop attempt to avoid extra dict copies around branching
Summary

In the python code, we attempt to avoid unnecessary dict copies when gathering
copy information. However that logic is wobbly and I keep running into case
where independent branches affects each others.

With the current code we can't ensure we are the only "user" of dict when
dealing with merge.

This caused havoc in the next series on tests I am about to introduce.

So for now I am disabling the faulty optimisation. I believe we will need a
dedicated overlay to deal with the "copy on write logic" to have something
correct. I am also hoping to find time to build dedicated test case for this
category of problem instead of relying on side effect in other tests. However
for now I am focussing on another issue.

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

marmoute created this revision.Dec 15 2020, 4:07 AM
baymax updated this revision to Diff 24371.Dec 17 2020, 4:03 PM

✅ refresh by Heptapod after a successful CI run (🐙 💚)

baymax updated this revision to Diff 24398.Dec 18 2020, 9:12 AM

✅ refresh by Heptapod after a successful CI run (🐙 💚)

baymax updated this revision to Diff 24405.Dec 18 2020, 9:31 AM

✅ refresh by Heptapod after a successful CI run (🐙 💚)

baymax updated this revision to Diff 24413.Dec 18 2020, 9:54 AM

✅ refresh by Heptapod after a successful CI run (🐙 💚)

baymax updated this revision to Diff 24451.Dec 21 2020, 3:00 AM

✅ refresh by Heptapod after a successful CI run (🐙 💚)

pulkit accepted this revision.Dec 28 2020, 3:13 AM
This revision is now accepted and ready to land.Dec 28 2020, 3:13 AM

Revision Contents
Changeset List

PathPackages
Mmercurial/copies.py (13 lines)

Diff 24525

mercurial/copies.py

children_count[parent_rev] = remaining_children children_count[parent_rev] = remaining_children
if remaining_children: if remaining_children:
copies = all_copies.get(parent_rev, None) copies = all_copies.get(parent_rev, None)
else: else:
copies = all_copies.pop(parent_rev, None) copies = all_copies.pop(parent_rev, None)
if copies is None: if copies is None:
# this is a root # this is a root
copies = {} newcopies = copies = {}
elif remaining_children:
newcopies = copies.copy()
else:
newcopies = copies newcopies = copies
# chain the data in the edge with the existing data # chain the data in the edge with the existing data
if changes is not None: if changes is not None:
childcopies = {} childcopies = {}
if parent == 1: if parent == 1:
childcopies = changes.copied_from_p1 childcopies = changes.copied_from_p1
elif parent == 2: elif parent == 2:
childcopies = changes.copied_from_p2 childcopies = changes.copied_from_p2
if childcopies: if childcopies:
newcopies = copies.copy() newcopies = copies.copy()
for dest, source in pycompat.iteritems(childcopies): for dest, source in pycompat.iteritems(childcopies):
prev = copies.get(source) prev = copies.get(source)
if prev is not None and prev[1] is not None: if prev is not None and prev[1] is not None:
source = prev[1] source = prev[1]
newcopies[dest] = (current_rev, source) newcopies[dest] = (current_rev, source)
assert newcopies is not copies assert newcopies is not copies
if changes.removed: if changes.removed:
if newcopies is copies:
newcopies = copies.copy()
for f in changes.removed: for f in changes.removed:
if f in newcopies: if f in newcopies:
if newcopies is copies: if newcopies is copies:
# copy on write to avoid affecting potential other # copy on write to avoid affecting potential other
# branches. when there are no other branches, this # branches. when there are no other branches, this
# could be avoided. # could be avoided.
newcopies = copies.copy() newcopies = copies.copy()
newcopies[f] = (current_rev, None) newcopies[f] = (current_rev, None)
# check potential need to combine the data from another parent (for # check potential need to combine the data from another parent (for
# that child). See comment below for details. # that child). See comment below for details.
if current_copies is None: if current_copies is None:
current_copies = newcopies current_copies = newcopies
elif current_copies is newcopies:
# nothing to merge:
pass
else: else:
# we are the second parent to work on c, we need to merge our # we are the second parent to work on c, we need to merge our
# work with the other. # work with the other.
# #
# In case of conflict, parent 1 take precedence over parent 2. # In case of conflict, parent 1 take precedence over parent 2.
# This is an arbitrary choice made anew when implementing # This is an arbitrary choice made anew when implementing
# changeset based copies. It was made without regards with # changeset based copies. It was made without regards with
# potential filelog related behavior. # potential filelog related behavior.