This is an archive of the discontinued Mercurial Phabricator instance.

Differential D2881

hgweb: refactor multirequest to be a dict of lists
ClosedPublic

Authored by indygreg on Mar 16 2018, 12:41 PM.

Details

Reviewers
yuja
Group Reviewers
hg-reviewers
Commits
rHG44467a4d472f: hgweb: refactor multirequest to be a dict of lists
Summary

... instead of a list of 2-tuples.

This makes key lookups faster. The only downside is we lose total
ordering of all entries. But we weren't relying on that before, so
it's no loss.

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

indygreg created this revision.Mar 16 2018, 12:41 PM
yuja accepted this revision.Mar 19 2018, 9:40 AM
This revision is now accepted and ready to land.Mar 19 2018, 9:40 AM
yuja added a comment.Mar 19 2018, 9:42 AM

Queued, thanks.

mercurial/hgweb/request.py
30–31

Deleted this comment in flight.

49

The behavior of len() changed, but I don't think that matters. The new
behavior should be fine since multidict isn't iterable.

Closed by commit rHG44467a4d472f: hgweb: refactor multirequest to be a dict of lists (authored by indygreg). · Explain WhyMar 19 2018, 9:46 AM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mmercurial/hgweb/request.py (45 lines)

Diff 7114

mercurial/hgweb/request.py

class multidict(object): class multidict(object):
"""A dict like object that can store multiple values for a key. """A dict like object that can store multiple values for a key.
Used to store parsed request parameters. Used to store parsed request parameters.
This is inspired by WebOb's class of the same name. This is inspired by WebOb's class of the same name.
""" """
def __init__(self): def __init__(self):
# Stores (key, value) 2-tuples. This isn't the most efficient. But we self._items = {}
yujaUnsubmitted
Not Done

Deleted this comment in flight.

yuja: Deleted this comment in flight.
# don't rely on parameters that much, so it shouldn't be a perf issue.
# we can always add dict for fast lookups.
self._items = []
def __getitem__(self, key): def __getitem__(self, key):
"""Returns the last set value for a key.""" """Returns the last set value for a key."""
for k, v in reversed(self._items): return self._items[key][-1]
if k == key:
return v
raise KeyError(key)
def __setitem__(self, key, value): def __setitem__(self, key, value):
"""Replace a values for a key with a new value.""" """Replace a values for a key with a new value."""
try: self._items[key] = [value]
del self[key]
except KeyError:
pass
self._items.append((key, value))
def __delitem__(self, key): def __delitem__(self, key):
"""Delete all values for a key.""" """Delete all values for a key."""
oldlen = len(self._items) del self._items[key]
self._items[:] = [(k, v) for k, v in self._items if k != key]
if oldlen == len(self._items):
raise KeyError(key)
def __contains__(self, key): def __contains__(self, key):
return any(k == key for k, v in self._items) return key in self._items
def __len__(self): def __len__(self):
return len(self._items) return len(self._items)
yujaUnsubmitted
Not Done

The behavior of len() changed, but I don't think that matters. The new
behavior should be fine since multidict isn't iterable.

yuja: The behavior of `len()` changed, but I don't think that matters. The new behavior should be…
def get(self, key, default=None): def get(self, key, default=None):
try: try:
return self.__getitem__(key) return self.__getitem__(key)
except KeyError: except KeyError:
return default return default
def add(self, key, value): def add(self, key, value):
"""Add a new value for a key. Does not replace existing values.""" """Add a new value for a key. Does not replace existing values."""
self._items.append((key, value)) self._items.setdefault(key, []).append(value)
def getall(self, key): def getall(self, key):
"""Obtains all values for a key.""" """Obtains all values for a key."""
return [v for k, v in self._items if k == key] return self._items.get(key, [])
def getone(self, key): def getone(self, key):
"""Obtain a single value for a key. """Obtain a single value for a key.
Raises KeyError if key not defined or it has multiple values set. Raises KeyError if key not defined or it has multiple values set.
""" """
vals = self.getall(key) vals = self._items[key]
if not vals:
raise KeyError(key)
if len(vals) > 1: if len(vals) > 1:
raise KeyError('multiple values for %r' % key) raise KeyError('multiple values for %r' % key)
return vals[0] return vals[0]
def asdictoflists(self): def asdictoflists(self):
d = {} return {k: list(v) for k, v in self._items.iteritems()}
for k, v in self._items:
if k in d:
d[k].append(v)
else:
d[k] = [v]
return d
@attr.s(frozen=True) @attr.s(frozen=True)
class parsedrequest(object): class parsedrequest(object):
"""Represents a parsed WSGI request. """Represents a parsed WSGI request.
Contains both parsed parameters as well as a handle on the input stream. Contains both parsed parameters as well as a handle on the input stream.
""" """