Remove Atomos as a dependency in favor of readerwriterlock (#624)

* Remove Atomos as a dependency in favor of readerwriterlock

* update PR link

* Oops

* Install toml extra for coverage

* Slight changes

* Fool of a took

Author: chrisrink10

CHANGELOG.md

@@ -12,7 +12,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
  * PyTest is now an optional extra dependency, rather than a required dependency (#622)
 
 ### Removed
- * Removed Click as a dependency in favor of builtin `argparse` (#622)
+ * Removed Click as a dependency in favor of builtin `argparse` (#622, #624)
+ * Removed Atomos as a dependency in favor of `readerwriterlock` (#624)
 
 ## [v0.1.dev15]
 ### Added

pyproject.toml

@@ -31,12 +31,12 @@ include = ["README.md", "LICENSE"]
 [tool.poetry.dependencies]
 python = "^3.6"
 astor = ">=0.8"
-atomos = "*"
 attrs = "*"
 immutables = "~0.15"
-prompt-toolkit = "~=3.0.0"
+prompt-toolkit = "~3.0.0"
 pyrsistent = "*"
 python-dateutil = "*"
+readerwriterlock = "~1.0.8"
 
 [tool.poetry.dev-dependencies]
 black = "*"
@@ -55,7 +55,7 @@ pygments = ["pygments"]
 pytest = ["pytest"]
 
 [tool.poetry.scripts]
-basilisp = "basilisp.cli:cli"
+basilisp = "basilisp.cli:invoke_cli"
 
 [tool.poetry.plugins.pytest11]
 basilisp_test_runner = "basilisp.testrunner"

src/basilisp/lang/atom.py

@@ -1,7 +1,6 @@
-import threading
 from typing import Callable, Generic, Optional, TypeVar
 
-import atomos.atom
+from readerwriterlock.rwlock import RWLockFair
 
 from basilisp.lang.interfaces import IDeref, IPersistentMap
 from basilisp.lang.reference import ReferenceBase
@@ -10,22 +9,41 @@
 
 
 class Atom(IDeref[T], ReferenceBase, Generic[T]):
-    __slots__ = ("_atom", "_meta", "_lock")
+    __slots__ = ("_meta", "_state", "_rlock", "_wlock")
 
     # pylint: disable=assigning-non-slot
     def __init__(self, state: T) -> None:
-        self._atom = atomos.atom.Atom(state)
         self._meta: Optional[IPersistentMap] = None
-        self._lock = threading.Lock()
-
-    def compare_and_set(self, old, new):
-        return self._atom.compare_and_set(old, new)
+        self._state = state
+        lock = RWLockFair()
+        self._rlock = lock.gen_rlock()
+        self._wlock = lock.gen_wlock()
+
+    def compare_and_set(self, old: T, new: T) -> bool:
+        """Compare the current state of the Atom to `old`. If the value is the same,
+        atomically set the value of the state of Atom to `new`. Return True if the
+        value was swapped. Return False otherwise."""
+        with self._wlock:
+            if self._state != old:
+                return False
+            self._state = new
+            return True
 
     def deref(self) -> T:
-        return self._atom.deref()
+        """Return the state stored within the Atom."""
+        with self._rlock:
+            return self._state
 
     def reset(self, v: T) -> T:
-        return self._atom.reset(v)
+        """Reset the state of the Atom to `v` without regard to the current value."""
+        with self._wlock:
+            self._state = v
+            return v
 
     def swap(self, f: Callable[..., T], *args, **kwargs) -> T:
-        return self._atom.swap(f, *args, **kwargs)
+        """Atomically swap the state of the Atom to the return value of
+        `f(old, *args, **kwargs)`, returning the new value."""
+        with self._wlock:
+            newval = f(self._state, *args, **kwargs)
+            self._state = newval
+            return newval

src/basilisp/lang/reference.py

@@ -1,6 +1,7 @@
-import threading
 from typing import Callable, Optional
 
+from readerwriterlock.rwlock import Lockable
+
 from basilisp.lang.interfaces import IPersistentMap, IReference
 
 try:
@@ -21,20 +22,21 @@ class ReferenceBase(IReference):
 
     Consumers must have a `_lock` and `_meta` property defined."""
 
-    _lock: threading.Lock
+    _rlock: Lockable
+    _wlock: Lockable
     _meta: Optional[IPersistentMap]
 
     @property
     def meta(self) -> Optional[IPersistentMap]:
-        with self._lock:
+        with self._rlock:
             return self._meta
 
     def alter_meta(self, f: AlterMeta, *args) -> IPersistentMap:
-        with self._lock:
+        with self._wlock:
             self._meta = f(self._meta, *args)
             return self._meta
 
     def reset_meta(self, meta: IPersistentMap) -> IPersistentMap:
-        with self._lock:
+        with self._wlock:
             self._meta = meta
             return meta

src/basilisp/lang/runtime.py

@@ -31,6 +31,8 @@
     cast,
 )
 
+from readerwriterlock.rwlock import RWLockFair
+
 from basilisp.lang import keyword as kw
 from basilisp.lang import list as llist
 from basilisp.lang import map as lmap
@@ -223,7 +225,8 @@ class Var(IDeref, ReferenceBase):
         "_is_bound",
         "_tl",
         "_meta",
-        "_lock",
+        "_rlock",
+        "_wlock",
     )
 
     def __init__(
@@ -240,7 +243,9 @@ def __init__(
         self._is_bound = False
         self._tl = None
         self._meta = meta
-        self._lock = threading.Lock()
+        lock = RWLockFair()
+        self._rlock = lock.gen_rlock()
+        self._wlock = lock.gen_wlock()
 
         if dynamic:
             self._tl = _VarBindings()
@@ -268,10 +273,6 @@ def name(self) -> sym.Symbol:
     def dynamic(self) -> bool:
         return self._dynamic
 
-    @dynamic.setter
-    def dynamic(self, is_dynamic: bool):
-        self._dynamic = is_dynamic
-
     @property
     def is_private(self) -> Optional[bool]:
         if self._meta is not None:
@@ -282,20 +283,25 @@ def is_private(self) -> Optional[bool]:
     def is_bound(self) -> bool:
         return self._is_bound or self.is_thread_bound
 
+    def _set_root(self, val) -> None:
+        # Private setter which does not include lock so it can be used
+        # by other properties and methods which already manage the lock.
+        self._is_bound = True
+        self._root = val
+
     @property
     def root(self):
-        with self._lock:
+        with self._rlock:
             return self._root
 
     @root.setter
     def root(self, val):
-        with self._lock:
-            self._is_bound = True
-            self._root = val
+        with self._wlock:
+            self._set_root(val)
 
     def alter_root(self, f, *args):
-        with self._lock:
-            self._root = f(self._root, *args)
+        with self._wlock:
+            self._set_root(f(self._root, *args))
 
     def push_bindings(self, val):
         if self._tl is None:
@@ -316,22 +322,24 @@ def is_thread_bound(self):
 
     @property
     def value(self):
-        if self._dynamic:
-            assert self._tl is not None
-            if len(self._tl.bindings) > 0:
-                return self._tl.bindings[-1]
-        return self.root
+        with self._rlock:
+            if self._dynamic:
+                assert self._tl is not None
+                if len(self._tl.bindings) > 0:
+                    return self._tl.bindings[-1]
+            return self._root
 
     @value.setter
     def value(self, v):
-        if self._dynamic:
-            assert self._tl is not None
-            if len(self._tl.bindings) > 0:
-                self._tl.bindings[-1] = v
-            else:
-                self.push_bindings(v)
-            return
-        self.root = v
+        with self._wlock:
+            if self._dynamic:
+                assert self._tl is not None
+                if len(self._tl.bindings) > 0:
+                    self._tl.bindings[-1] = v
+                else:
+                    self.push_bindings(v)
+                return
+            self._set_root(v)
 
     @staticmethod
     def intern(
@@ -496,7 +504,8 @@ class Namespace(ReferenceBase):
         "_name",
         "_module",
         "_meta",
-        "_lock",
+        "_rlock",
+        "_wlock",
         "_interns",
         "_refers",
         "_aliases",
@@ -509,7 +518,9 @@ def __init__(self, name: sym.Symbol, module: BasilispModule = None) -> None:
         self._module = Maybe(module).or_else(lambda: _new_module(name.as_python_sym()))
 
         self._meta: Optional[IPersistentMap] = None
-        self._lock = threading.Lock()
+        lock = RWLockFair()
+        self._rlock = lock.gen_rlock()
+        self._wlock = lock.gen_wlock()
 
         self._aliases: NamespaceMap = lmap.PersistentMap.empty()
         self._imports: ModuleMap = lmap.map(
@@ -545,36 +556,36 @@ def module(self, m: BasilispModule):
     def aliases(self) -> NamespaceMap:
         """A mapping between a symbolic alias and another Namespace. The
         fully qualified name of a namespace is also an alias for itself."""
-        with self._lock:
+        with self._rlock:
             return self._aliases
 
     @property
     def imports(self) -> ModuleMap:
         """A mapping of names to Python modules imported into the current
         namespace."""
-        with self._lock:
+        with self._rlock:
             return self._imports
 
     @property
     def import_aliases(self) -> AliasMap:
         """A mapping of a symbolic alias and a Python module name."""
-        with self._lock:
+        with self._rlock:
             return self._import_aliases
 
     @property
     def interns(self) -> VarMap:
         """A mapping between a symbolic name and a Var. The Var may point to
         code, data, or nothing, if it is unbound. Vars in `interns` are
         interned in _this_ namespace."""
-        with self._lock:
+        with self._rlock:
             return self._interns
 
     @property
     def refers(self) -> VarMap:
         """A mapping between a symbolic name and a Var. Vars in refers are
         interned in another namespace and are only referred to without an
         alias in this namespace."""
-        with self._lock:
+        with self._rlock:
             return self._refers
 
     def __repr__(self):
@@ -605,41 +616,41 @@ def require(self, ns_name: str, *aliases: sym.Symbol) -> BasilispModule:
 
     def add_alias(self, namespace: "Namespace", *aliases: sym.Symbol) -> None:
         """Add Symbol aliases for the given Namespace."""
-        with self._lock:
+        with self._wlock:
             new_m = self._aliases
             for alias in aliases:
                 new_m = new_m.assoc(alias, namespace)
             self._aliases = new_m
 
     def get_alias(self, alias: sym.Symbol) -> "Optional[Namespace]":
         """Get the Namespace aliased by Symbol or None if it does not exist."""
-        with self._lock:
+        with self._rlock:
             return self._aliases.val_at(alias, None)
 
     def remove_alias(self, alias: sym.Symbol) -> None:
         """Remove the Namespace aliased by Symbol. Return None."""
-        with self._lock:
+        with self._wlock:
             self._aliases = self._aliases.dissoc(alias)
 
     def intern(self, sym: sym.Symbol, var: Var, force: bool = False) -> Var:
         """Intern the Var given in this namespace mapped by the given Symbol.
         If the Symbol already maps to a Var, this method _will not overwrite_
         the existing Var mapping unless the force keyword argument is given
         and is True."""
-        with self._lock:
+        with self._wlock:
             old_var = self._interns.val_at(sym, None)
             if old_var is None or force:
                 self._interns = self._interns.assoc(sym, var)
             return self._interns.val_at(sym)
 
     def unmap(self, sym: sym.Symbol) -> None:
-        with self._lock:
+        with self._wlock:
             self._interns = self._interns.dissoc(sym)
 
     def find(self, sym: sym.Symbol) -> Optional[Var]:
         """Find Vars mapped by the given Symbol input or None if no Vars are
         mapped by that Symbol."""
-        with self._lock:
+        with self._rlock:
             v = self._interns.val_at(sym, None)
             if v is None:
                 return self._refers.val_at(sym, None)
@@ -648,7 +659,7 @@ def find(self, sym: sym.Symbol) -> Optional[Var]:
     def add_import(self, sym: sym.Symbol, module: Module, *aliases: sym.Symbol) -> None:
         """Add the Symbol as an imported Symbol in this Namespace. If aliases are given,
         the aliases will be applied to the"""
-        with self._lock:
+        with self._wlock:
             self._imports = self._imports.assoc(sym, module)
             if aliases:
                 m = self._import_aliases
@@ -662,7 +673,7 @@ def get_import(self, sym: sym.Symbol) -> Optional[BasilispModule]:
 
         First try to resolve a module directly with the given name. If no module
         can be resolved, attempt to resolve the module using import aliases."""
-        with self._lock:
+        with self._rlock:
             mod = self._imports.val_at(sym, None)
             if mod is None:
                 alias = self._import_aliases.get(sym, None)
@@ -674,17 +685,17 @@ def get_import(self, sym: sym.Symbol) -> Optional[BasilispModule]:
     def add_refer(self, sym: sym.Symbol, var: Var) -> None:
         """Refer var in this namespace under the name sym."""
         if not var.is_private:
-            with self._lock:
+            with self._wlock:
                 self._refers = self._refers.assoc(sym, var)
 
     def get_refer(self, sym: sym.Symbol) -> Optional[Var]:
         """Get the Var referred by Symbol or None if it does not exist."""
-        with self._lock:
+        with self._rlock:
             return self._refers.val_at(sym, None)
 
     def refer_all(self, other_ns: "Namespace") -> None:
         """Refer all the Vars in the other namespace."""
-        with self._lock:
+        with self._wlock:
             final_refers = self._refers
             for s, var in other_ns.interns.items():
                 if not var.is_private:

tests/basilisp/atom_test.py

@@ -22,7 +22,7 @@ def test_atom():
     assert vec.v(1, 2) == a.swap(lambda v, e: v.cons(e), 2)
     assert vec.v(1, 2) == a.deref()
 
-    assert vec.v(1, 2) == a.reset(lmap.PersistentMap.empty())
+    assert lmap.PersistentMap.empty() == a.reset(lmap.PersistentMap.empty())
     assert lmap.PersistentMap.empty() == a.deref()