Convert Map to use immutables.Map rather than pyrsistent (#559)

Convert `basilisp.lang.map.Map` and `basilisp.lang.set.Set` to use `immutables.Map` under the hood, rather than the Python-only implementation provided by `pyrsistent`, which should offer some decent performance improvements for both types.

As another performance improvement, most of the basic runtime functions like `first`, `rest`, `cons`, et al were converted to single dispatch (using `@functools.singledispatch`), which should be a little faster than a series of `isinstance` checks.

Author: chrisrink10

CHANGELOG.md

@@ -17,6 +17,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
  * Added support for Volatiles (#460)
  * Add JSON encoder and decoder in `basilisp.json` namespace (#484)
 
+### Changed
+ * Basilisp set and map types are now backed by the HAMT provided by `immutables` (#557)
+
 ### Fixed
  * Fixed a bug where the Basilisp AST nodes for return values of `deftype` members could be marked as _statements_ rather than _expressions_, resulting in an incorrect `nil` return (#523)
  * Fixed a bug where `defonce` would throw a Python SyntaxError due to a superfluous `global` statement in the generated Python (#525)
@@ -27,6 +30,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
  * Fixed a bug where not all builtin Basilisp types could be pickled (#518)
  * Fixed a bug where `deftype` forms could not be created interfaces declared not at the top-level of a code block in a namespace (#376)
  * Fixed multiple bugs relating to symbol resolution of `import`ed symbols in various contexts (#544)
+ * Fixed a bug where the `=` function did not respect the equality partition for various builtin collection types (#556)
 
 ## [v0.1.dev13] - 2020-03-16
 ### Added

Pipfile

@@ -26,6 +26,7 @@ pyfunctional = "*"
 pyrsistent = "*"
 python-dateutil = "*"
 pytest = "*"
+immutables = "*"
 
 [requires]
 python_version = "3.6"

Pipfile.lock

@@ -25,6 +25,7 @@
     "atomos",
     "attrs",
     "click",
+    "immutables",
     "prompt-toolkit>=3.0.0",
     "pyfunctional",
     "pyrsistent",

setup.py

@@ -241,10 +241,7 @@
     :arglists '([coll])}
   count
   (fn count [coll]
-    (try
-      (python/len coll)
-      (catch TypeError _
-        (count (apply vector coll))))))
+    (basilisp.lang.runtime/count coll)))
 
 (def
   ^{:doc      "Returns a basilisp.lang.exception/ExceptionInfo instance with
@@ -1098,6 +1095,11 @@
   [x]
   (python/callable x))
 
+(defn indexed?
+  "Return true if elements of x can be accessed by index efficiently."
+  [x]
+  (instance? basilisp.lang.interfaces/IIndexed x))
+
 (def ^{:doc "Return true if x is an integer.
 
              Note that unlike Clojure, Basilisp uses Python integers and

src/basilisp/core.lpy

@@ -37,6 +37,20 @@ def deref(
 
 
 class ICounted(ABC):
+    """ICounted types can produce their length in constant time.
+
+    All of the builtin collections are ICounted, except Lists whose length is
+    determined by counting all of the elements in the list in linear time."""
+
+    __slots__ = ()
+
+
+class IIndexed(ICounted, ABC):
+    """IIndexed types can be accessed by index.
+
+    Of the builtin collections, only Vectors are IIndexed. IIndexed types respond
+    True to the `indexed?` predicate."""
+
     __slots__ = ()
 
 
@@ -107,6 +121,12 @@ def reset_meta(self, meta: "IPersistentMap") -> "IPersistentMap":
 
 
 class IReversible(Generic[T]):
+    """IReversible types can produce a sequences of their elements in reverse in
+    constant time.
+
+    Of the builtin collections, only Vectors are IReversible. IIndexed types respond
+    True to the `reversible` predicate."""
+
     __slots__ = ()
 
     @abstractmethod
@@ -115,6 +135,11 @@ def rseq(self) -> "ISeq[T]":
 
 
 class ISeqable(Iterable[T]):
+    """ISeqable types can produce sequences of their elements, but are not ISeqs.
+
+    All of the builtin collections are ISeqable, except Lists which directly
+    implement ISeq. Values of type ISeqable respond True to the `seqable?` predicate."""
+
     __slots__ = ()
 
     @abstractmethod
@@ -123,6 +148,11 @@ def seq(self) -> "ISeq[T]":
 
 
 class ISequential(ABC):
+    """ISequential is a marker interface for sequential types.
+
+    Lists and Vectors are both considered ISequential and respond True to the
+    `sequential?` predicate."""
+
     __slots__ = ()
 
 
@@ -222,7 +252,7 @@ def disj(self: T_set, *elems: T) -> T_set:
 class IPersistentVector(
     Sequence[T],
     IAssociative[int, T],
-    ICounted,
+    IIndexed,
     IReversible[T],
     ISequential,
     IPersistentStack[T],
@@ -243,6 +273,10 @@ def seq(self) -> "ISeq[T]":  # type: ignore[override]
 
 
 class IRecord(ILispObject):
+    """IRecord is a marker interface for types def'ed by `defrecord` forms.
+
+    All types created by `defrecord` are automatically marked with IRecord."""
+
     __slots__ = ()
 
     @classmethod
@@ -347,4 +381,8 @@ def __iter__(self):
 
 
 class IType(ABC):
+    """IType is a marker interface for types def'ed by `deftype` forms.
+
+    All types created by `deftype` are automatically marked with IType."""
+
     __slots__ = ()

src/basilisp/lang/interfaces.py

@@ -1,7 +1,7 @@
 from builtins import map as pymap
-from typing import Callable, Iterable, Mapping, Optional, TypeVar, Union, cast
+from typing import Callable, Iterable, Mapping, Optional, Tuple, TypeVar, Union, cast
 
-from pyrsistent import PMap, pmap  # noqa # pylint: disable=unused-import
+from immutables import Map as _Map
 
 from basilisp.lang.interfaces import (
     ILispObject,
@@ -25,16 +25,18 @@
 
 
 class Map(IPersistentMap[K, V], ILispObject, IWithMeta):
-    """Basilisp Map. Delegates internally to a pyrsistent.PMap object.
+    """Basilisp Map. Delegates internally to a immutables.Map object.
     Do not instantiate directly. Instead use the m() and map() factory
     methods below."""
 
     __slots__ = ("_inner", "_meta")
 
     def __init__(
-        self, wrapped: "PMap[K, V]", meta: Optional[IPersistentMap] = None
+        self,
+        members: Union[Mapping[K, V], Iterable[Tuple[K, V]]],
+        meta: Optional[IPersistentMap] = None,
     ) -> None:
-        self._inner = wrapped
+        self._inner = _Map(members)
         self._meta = meta
 
     def __call__(self, key, default=None):
@@ -50,7 +52,7 @@ def __eq__(self, other):
             return NotImplemented
         if len(self._inner) != len(other):
             return False
-        return dict(self._inner.iteritems()) == dict(other.items())
+        return self._inner == other
 
     def __getitem__(self, item):
         return self._inner[item]
@@ -59,14 +61,14 @@ def __hash__(self):
         return hash(self._inner)
 
     def __iter__(self):
-        yield from self._inner.iterkeys()
+        return iter(self._inner)
 
     def __len__(self):
         return len(self._inner)
 
     def _lrepr(self, **kwargs):
         return _map_lrepr(
-            self._inner.iteritems, start="{", end="}", meta=self._meta, **kwargs
+            self._inner.items, start="{", end="}", meta=self._meta, **kwargs
         )
 
     @property
@@ -77,24 +79,22 @@ def with_meta(self, meta: Optional[IPersistentMap]) -> "Map":
         return Map(self._inner, meta=meta)
 
     def assoc(self, *kvs):
-        m = self._inner.evolver()
-        for k, v in partition(kvs, 2):
-            m[k] = v
-        return Map(m.persistent())
+        with self._inner.mutate() as m:
+            for k, v in partition(kvs, 2):
+                m[k] = v
+            return Map(m.finish())
 
     def contains(self, k):
-        if k in self._inner:
-            return True
-        return False
+        return k in self._inner
 
     def dissoc(self, *ks):
-        m = self._inner.evolver()
-        for k in ks:
-            try:
-                del m[k]
-            except KeyError:
-                pass
-        return Map(m.persistent())
+        with self._inner.mutate() as m:
+            for k in ks:
+                try:
+                    del m[k]
+                except KeyError:
+                    pass
+            return Map(m.finish())
 
     def entry(self, k):
         v = self._inner.get(k, cast("V", _ENTRY_SENTINEL))
@@ -106,14 +106,17 @@ def val_at(self, k, default=None):
         return self._inner.get(k, default)
 
     def update(self, *maps: Mapping[K, V]) -> "Map":
-        m: PMap = self._inner.update(*maps)
+        m: _Map = self._inner.update(*(m.items() for m in maps))
         return Map(m)
 
     def update_with(
         self, merge_fn: Callable[[V, V], V], *maps: Mapping[K, V]
     ) -> "Map[K, V]":
-        m: PMap = self._inner.update_with(merge_fn, *maps)
-        return Map(m)
+        with self._inner.mutate() as m:
+            for map in maps:
+                for k, v in map.items():
+                    m.set(k, merge_fn(m[k], v) if k in m else v)
+            return Map(m.finish())
 
     def cons(  # type: ignore[override]
         self,
@@ -124,51 +127,53 @@ def cons(  # type: ignore[override]
             Mapping[K, V],
         ],
     ) -> "Map[K, V]":
-        e = self._inner.evolver()
-        try:
-            for elem in elems:
-                if isinstance(elem, (IPersistentMap, Mapping)):
-                    for k, v in elem.items():
-                        e.set(k, v)
-                elif isinstance(elem, IMapEntry):
-                    e.set(elem.key, elem.value)
-                elif elem is None:
-                    continue
-                else:
-                    # This block leniently allows nearly any 2 element sequential
-                    # type including Vectors, Python lists, and Python tuples.
-                    entry: IMapEntry[K, V] = MapEntry.from_vec(elem)
-                    e.set(entry.key, entry.value)
-        except (TypeError, ValueError):
-            raise ValueError(
-                "Argument to map conj must be another Map or castable to MapEntry"
-            )
-        else:
-            return Map(e.persistent(), meta=self.meta)
+        with self._inner.mutate() as m:
+            try:
+                for elem in elems:
+                    if isinstance(elem, (IPersistentMap, Mapping)):
+                        for k, v in elem.items():
+                            m.set(k, v)
+                    elif isinstance(elem, IMapEntry):
+                        m.set(elem.key, elem.value)
+                    elif elem is None:
+                        continue
+                    else:
+                        entry: IMapEntry[K, V] = MapEntry.from_vec(elem)
+                        m.set(entry.key, entry.value)
+            except (TypeError, ValueError):
+                raise ValueError(
+                    "Argument to map conj must be another Map or castable to MapEntry"
+                )
+            else:
+                return Map(m.finish(), meta=self.meta)
 
     @staticmethod
     def empty() -> "Map":
         return m()
 
     def seq(self) -> ISeq[IMapEntry[K, V]]:
-        return sequence(MapEntry.of(k, v) for k, v in self._inner.iteritems())
+        return sequence(MapEntry.of(k, v) for k, v in self._inner.items())
 
 
 def map(kvs: Mapping[K, V], meta=None) -> Map[K, V]:  # pylint:disable=redefined-builtin
     """Creates a new map."""
-    return Map(pmap(initial=kvs), meta=meta)
+    # For some reason, creating a new `immutables.Map` instance from an existing
+    # `basilisp.lang.map.Map` instance causes issues because the `__iter__` returns
+    # only the keys rather than tuple of key/value pairs, even though it adheres to
+    # the `Mapping` protocol. Passing the `.items()` directly bypasses this problem.
+    return Map(kvs.items(), meta=meta)
 
 
 def m(**kvs) -> Map[str, V]:
     """Creates a new map from keyword arguments."""
-    return Map(pmap(initial=kvs))
+    return Map(kvs)
 
 
 def from_entries(entries: Iterable[MapEntry[K, V]]) -> Map[K, V]:
-    m = pmap().evolver()  # type: ignore
-    for entry in entries:
-        m.set(entry.key, entry.value)
-    return Map(m.persistent())
+    with _Map().mutate() as m:  # type: ignore[var-annotated]
+        for entry in entries:
+            m.set(entry.key, entry.value)
+        return Map(m.finish())
 
 
 def hash_map(*pairs) -> Map: