This is an archive of the discontinued Mercurial Phabricator instance.

Differential D8837

mergeresult: make actionmapping a dict of dict instead of dict of lists
ClosedPublic

Authored by pulkit on Jul 25 2020, 6:03 AM.

Details

Reviewers
indygreg
Group Reviewers
hg-reviewers
Commits
rHGe98f7c5babd7: mergeresult: make actionmapping a dict of dict instead of dict of lists
Summary

This makes deleting a specific filename entry faster and easier.

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

pulkit created this revision.Jul 25 2020, 6:03 AM
indygreg accepted this revision.Aug 2 2020, 1:35 PM
indygreg added a subscriber: indygreg.
indygreg added inline comments.
mercurial/merge.py
597

Having the separate index is useful and I think the new refactored data structure in this patch is better. But I still don't like having the (data, message) stored redundantly, as it introduces a synchronization problem.

What do you think about changing self._actionmapping to be a dict of sets of filenames. The main reason not to do this would be if the extra dict lookup in self._filemapping to retrieve (data, message) adds overhead. The many callers of getactions() could be performance sensitive, I'm not sure...

This revision is now accepted and ready to land.Aug 2 2020, 1:35 PM

Revision Contents
Changeset List

PathPackages
Mmercurial/merge.py (27 lines)

Diff 22220

mercurial/merge.py

""" """
filemapping: dict of filename as keys and action related info as values filemapping: dict of filename as keys and action related info as values
diverge: mapping of source name -> list of dest name for diverge: mapping of source name -> list of dest name for
divergent renames divergent renames
renamedelete: mapping of source name -> list of destinations for files renamedelete: mapping of source name -> list of destinations for files
deleted on one side and renamed on other. deleted on one side and renamed on other.
commitinfo: dict containing data which should be used on commit commitinfo: dict containing data which should be used on commit
contains a filename -> info mapping contains a filename -> info mapping
actionmapping: dict of action names as keys and list of files and actionmapping: dict of action names as keys and values are dict of
related data as values filename as key and related data as values
""" """
self._filemapping = {} self._filemapping = {}
self._diverge = {} self._diverge = {}
self._renamedelete = {} self._renamedelete = {}
self._commitinfo = {} self._commitinfo = {}
self._actionmapping = collections.defaultdict(list) self._actionmapping = collections.defaultdict(dict)
def updatevalues(self, diverge, renamedelete, commitinfo): def updatevalues(self, diverge, renamedelete, commitinfo):
self._diverge = diverge self._diverge = diverge
self._renamedelete = renamedelete self._renamedelete = renamedelete
self._commitinfo = commitinfo self._commitinfo = commitinfo
def addfile(self, filename, action, data, message): def addfile(self, filename, action, data, message):
""" adds a new file to the mergeresult object """ adds a new file to the mergeresult object
filename: file which we are adding filename: file which we are adding
action: one of mergestatemod.ACTION_* action: one of mergestatemod.ACTION_*
data: a tuple of information like fctx and ctx related to this merge data: a tuple of information like fctx and ctx related to this merge
message: a message about the merge message: a message about the merge
""" """
# if the file already existed, we need to delete it's old # if the file already existed, we need to delete it's old
# entry form _actionmapping too # entry form _actionmapping too
if filename in self._filemapping: if filename in self._filemapping:
# TODO: this is inefficient
a, d, m = self._filemapping[filename] a, d, m = self._filemapping[filename]
self._actionmapping[a].remove((filename, d, m)) del self._actionmapping[a][filename]
self._filemapping[filename] = (action, data, message) self._filemapping[filename] = (action, data, message)
self._actionmapping[action].append((filename, data, message)) self._actionmapping[action][filename] = (data, message)
indygregUnsubmitted
Not Done

Having the separate index is useful and I think the new refactored data structure in this patch is better. But I still don't like having the (data, message) stored redundantly, as it introduces a synchronization problem.

What do you think about changing self._actionmapping to be a dict of sets of filenames. The main reason not to do this would be if the extra dict lookup in self._filemapping to retrieve (data, message) adds overhead. The many callers of getactions() could be performance sensitive, I'm not sure...

indygreg: Having the separate index is useful and I think the new refactored data structure in this patch…
def removefile(self, filename): def removefile(self, filename):
""" removes a file from the mergeresult object as the file might """ removes a file from the mergeresult object as the file might
not merging anymore """ not merging anymore """
action, data, message = self._filemapping[filename] action, data, message = self._filemapping[filename]
del self._filemapping[filename] del self._filemapping[filename]
# TODO: this is inefficient del self._actionmapping[action][filename]
self._actionmapping[action].remove((filename, data, message))
def getactions(self, actions): def getactions(self, actions):
""" get list of files which are marked with these actions """ get list of files which are marked with these actions
Returns a list of tuple of form (filename, data, message) Returns a list of tuple of form (filename, data, message)
""" """
res = [] res = []
for a in actions: for a in actions:
res.extend(self._actionmapping[a]) for f, (args, msg) in pycompat.iteritems(self._actionmapping[a]):
res.append((f, args, msg))
return res return res
@property @property
def actions(self): def actions(self):
return self._filemapping return self._filemapping
@property @property
def diverge(self): def diverge(self):
return self._diverge return self._diverge
@property @property
def renamedelete(self): def renamedelete(self):
return self._renamedelete return self._renamedelete
@property @property
def commitinfo(self): def commitinfo(self):
return self._commitinfo return self._commitinfo
@property @property
def actionsdict(self): def actionsdict(self):
""" returns a dictionary of actions to be perfomed with action as key """ returns a dictionary of actions to be perfomed with action as key
and a list of files and related arguments as values """ and a list of files and related arguments as values """
return self._actionmapping res = emptyactions()
for a, d in pycompat.iteritems(self._actionmapping):
for f, (args, msg) in pycompat.iteritems(d):
res[a].append((f, args, msg))
return res
def setactions(self, actions): def setactions(self, actions):
self._filemapping = actions self._filemapping = actions
self._actionmapping = collections.defaultdict(list) self._actionmapping = collections.defaultdict(dict)
for f, (act, data, msg) in pycompat.iteritems(self._filemapping): for f, (act, data, msg) in pycompat.iteritems(self._filemapping):
self._actionmapping[act].append((f, data, msg)) self._actionmapping[act][f] = data, msg
def updateactions(self, updates): def updateactions(self, updates):
for f, (a, data, msg) in pycompat.iteritems(updates): for f, (a, data, msg) in pycompat.iteritems(updates):
self.addfile(f, a, data, msg) self.addfile(f, a, data, msg)
def hasconflicts(self): def hasconflicts(self):
""" tells whether this merge resulted in some actions which can """ tells whether this merge resulted in some actions which can
result in conflicts or not """ result in conflicts or not """