D2881: hgweb: refactor multirequest to be a dict of lists

2018-03-19 Thread indygreg (Gregory Szorc)
This revision was automatically updated to reflect the committed changes.
Closed by commit rHG44467a4d472f: hgweb: refactor multirequest to be a dict of 
lists (authored by indygreg, committed by ).

CHANGED PRIOR TO COMMIT
  https://phab.mercurial-scm.org/D2881?vs=7078=7114#toc

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D2881?vs=7078=7114

REVISION DETAIL
  https://phab.mercurial-scm.org/D2881

AFFECTED FILES
  mercurial/hgweb/request.py

CHANGE DETAILS

diff --git a/mercurial/hgweb/request.py b/mercurial/hgweb/request.py
--- a/mercurial/hgweb/request.py
+++ b/mercurial/hgweb/request.py
@@ -28,39 +28,22 @@
 This is inspired by WebOb's class of the same name.
 """
 def __init__(self):
-# Stores (key, value) 2-tuples. This isn't the most efficient. But we
-# 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 = []
+self._items = {}
 
 def __getitem__(self, key):
 """Returns the last set value for a key."""
-for k, v in reversed(self._items):
-if k == key:
-return v
-
-raise KeyError(key)
+return self._items[key][-1]
 
 def __setitem__(self, key, value):
 """Replace a values for a key with a new value."""
-try:
-del self[key]
-except KeyError:
-pass
-
-self._items.append((key, value))
+self._items[key] = [value]
 
 def __delitem__(self, key):
 """Delete all values for a key."""
-oldlen = len(self._items)
-
-self._items[:] = [(k, v) for k, v in self._items if k != key]
-
-if oldlen == len(self._items):
-raise KeyError(key)
+del self._items[key]
 
 def __contains__(self, key):
-return any(k == key for k, v in self._items)
+return key in self._items
 
 def __len__(self):
 return len(self._items)
@@ -73,36 +56,26 @@
 
 def add(self, key, value):
 """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):
 """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):
 """Obtain a single value for a key.
 
 Raises KeyError if key not defined or it has multiple values set.
 """
-vals = self.getall(key)
-
-if not vals:
-raise KeyError(key)
+vals = self._items[key]
 
 if len(vals) > 1:
 raise KeyError('multiple values for %r' % key)
 
 return vals[0]
 
 def asdictoflists(self):
-d = {}
-for k, v in self._items:
-if k in d:
-d[k].append(v)
-else:
-d[k] = [v]
-
-return d
+return {k: list(v) for k, v in self._items.iteritems()}
 
 @attr.s(frozen=True)
 class parsedrequest(object):



To: indygreg, #hg-reviewers, yuja
Cc: mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel


D2881: hgweb: refactor multirequest to be a dict of lists

2018-03-19 Thread yuja (Yuya Nishihara)
yuja added a comment.


  Queued, thanks.

INLINE COMMENTS

> request.py:31
>  def __init__(self):
>  # Stores (key, value) 2-tuples. This isn't the most efficient. But we
>  # don't rely on parameters that much, so it shouldn't be a perf 
> issue.

Deleted this comment in flight.

> request.py:52
>  def __len__(self):
>  return len(self._items)
>  

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

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D2881

To: indygreg, #hg-reviewers, yuja
Cc: mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel


D2881: hgweb: refactor multirequest to be a dict of lists

2018-03-16 Thread indygreg (Gregory Szorc)
indygreg created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION 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.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D2881

AFFECTED FILES
  mercurial/hgweb/request.py

CHANGE DETAILS

diff --git a/mercurial/hgweb/request.py b/mercurial/hgweb/request.py
--- a/mercurial/hgweb/request.py
+++ b/mercurial/hgweb/request.py
@@ -31,36 +31,22 @@
 # Stores (key, value) 2-tuples. This isn't the most efficient. But we
 # 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 = []
+self._items = {}
 
 def __getitem__(self, key):
 """Returns the last set value for a key."""
-for k, v in reversed(self._items):
-if k == key:
-return v
-
-raise KeyError(key)
+return self._items[key][-1]
 
 def __setitem__(self, key, value):
 """Replace a values for a key with a new value."""
-try:
-del self[key]
-except KeyError:
-pass
-
-self._items.append((key, value))
+self._items[key] = [value]
 
 def __delitem__(self, key):
 """Delete all values for a key."""
-oldlen = len(self._items)
-
-self._items[:] = [(k, v) for k, v in self._items if k != key]
-
-if oldlen == len(self._items):
-raise KeyError(key)
+del self._items[key]
 
 def __contains__(self, key):
-return any(k == key for k, v in self._items)
+return key in self._items
 
 def __len__(self):
 return len(self._items)
@@ -73,36 +59,26 @@
 
 def add(self, key, value):
 """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):
 """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):
 """Obtain a single value for a key.
 
 Raises KeyError if key not defined or it has multiple values set.
 """
-vals = self.getall(key)
-
-if not vals:
-raise KeyError(key)
+vals = self._items[key]
 
 if len(vals) > 1:
 raise KeyError('multiple values for %r' % key)
 
 return vals[0]
 
 def asdictoflists(self):
-d = {}
-for k, v in self._items:
-if k in d:
-d[k].append(v)
-else:
-d[k] = [v]
-
-return d
+return {k: list(v) for k, v in self._items.iteritems()}
 
 @attr.s(frozen=True)
 class parsedrequest(object):



To: indygreg, #hg-reviewers
Cc: mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel