Align basilisp.core/deref with Clojure to use timeout in ms (#1008)

Hi,

could you please consider patch to update the `basilisp.core/deref`
timeout argument to us ms instead of secs. It addresses #1007

This is to align with the same in `clojure.core/deref`.

I have add a `promise` basilisp test and updated an existing `future`
one. The tests check that the timeout for an unfulfilled promise returns
within a range of 100ms. I believe this range provides sufficient buffer
to avoid interference from parallel processing on the host.

Thanks

---------

Co-authored-by: ikappaki <[email protected]>

Author: ikappaki

CHANGELOG.md

@@ -18,6 +18,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
  * Types created via `deftype` and `reify` may declare supertypes as abstract (taking precedence over true `abc.ABC` types) and specify their member list using `^:abstract-members` metadata (#942)
  * Load functions (`load`, `load-file`, `load-reader`, etc.) now return the value of the last form evaluated. (#984)
  * nREPL server no longer hangs waiting for client connections to close on exit (#1002)
+ * Align `basilisp.core/deref` with Clojure to by updating the timeout input argument to be in milliseconds instead of seconds (#1007)
 
 ### Fixed
  * Fix inconsistent behavior with `basilisp.core/with` when the `body` contains more than one form (#981)

src/basilisp/core.lpy

@@ -921,13 +921,13 @@
   "Dereference a delay, atom, promise, future, volatile, or Var and returns its contents.
 
   If ``o`` is an object implementing :py:class:`basilisp.lang.interfaces.IBlockingDeref`
-  and ``timeout-s`` and ``timeout-val`` are supplied, ``deref`` will wait at most
-  ``timeout-s`` seconds, returning ``timeout-val`` if ``timeout-s`` seconds elapse and
-  ``o`` has not returned."
+  (i.e. futures and promises) and ``timeout-ms`` and ``timeout-val`` are supplied, ``deref``
+  will wait at most ``timeout-ms`` milliseconds, returning ``timeout-val`` if ``timeout-ms``
+  milliseconds elapse and ``o`` has not returned."
   ([o]
    (basilisp.lang.runtime/deref o))
-  ([o timeout-s timeout-val]
-   (basilisp.lang.runtime/deref o timeout-s timeout-val)))
+  ([o timeout-ms timeout-val]
+   (basilisp.lang.runtime/deref o timeout-ms timeout-val)))
 
 (defn ^:inline compare-and-set!
   "Atomically set the value of ``atom`` to ``new-val`` if and only if ``old-val`` is

src/basilisp/lang/runtime.py

@@ -1454,20 +1454,23 @@ def partial_f(*inner_args, **inner_kwargs):
 
 
 @functools.singledispatch
-def deref(o, timeout_s=None, timeout_val=None):
+def deref(o, timeout_ms=None, timeout_val=None):
     """Dereference a Deref object and return its contents.
 
-    If o is an object implementing IBlockingDeref and timeout_s and
-    timeout_val are supplied, deref will wait at most timeout_s seconds,
-    returning timeout_val if timeout_s seconds elapse and o has not
+    If o is an object implementing IBlockingDeref and timeout_ms and
+    timeout_val are supplied, deref will wait at most timeout_ms milliseconds,
+    returning timeout_val if timeout_ms milliseconds elapse and o has not
     returned."""
     raise TypeError(f"Object of type {type(o)} cannot be dereferenced")
 
 
 @deref.register(IBlockingDeref)
 def _deref_blocking(
-    o: IBlockingDeref, timeout_s: Optional[float] = None, timeout_val=None
+    o: IBlockingDeref, timeout_ms: Optional[int] = None, timeout_val=None
 ):
+    timeout_s = None
+    if timeout_ms is not None:
+        timeout_s = timeout_ms / 1000 if timeout_ms != 0 else 0
     return o.deref(timeout_s, timeout_val)
 
 

tests/basilisp/contrib/nrepl_server_test.lpy

@@ -612,7 +612,7 @@
              :target #(nr/start-server! {:server* server* :nrepl-port-file filename :host "0.0.0.0"})
              :daemon true)
         (.start))
-      (let [server (deref server* 1 nil)]
+      (let [server (deref server* 1000 nil)]
         (is server)
         (try
           (is (bio/exists? filename))
@@ -636,7 +636,7 @@
                          :target #(nr/start-server! {:server* server*})
                          :daemon true)]
       (.start server-thread)
-      (let [server (deref server* 1 nil)]
+      (let [server (deref server* 1000 nil)]
         (is server)
         (let [[host_ port] (py->lisp (.-server-address server))]
           (binding [*nrepl-port* port]
@@ -648,5 +648,5 @@
               (let [shutdown-thread (future (doto server
                                               (.server-close))
                                             :done)
-                    status (deref shutdown-thread 1 :time-out)]
+                    status (deref shutdown-thread 1000 :time-out)]
                 (is (= :done status))))))))))

tests/basilisp/test_core_fns.lpy

@@ -1399,6 +1399,21 @@
   (is (= "lo w" (subs "hello world" 3 7)))
   (is (thrown? python/IndexError (subs "hello world" 12 3))))
 
+;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
+;; State Management Functions ;;
+;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
+
+(deftest promises-test
+  (testing "timeout expires on time"
+    (let [p (promise)
+          start (time/perf-counter)
+          ;; block for 100ms
+          ret (deref p 100 :timed-out)
+          end (time/perf-counter)
+          diff (- end start)]
+      (is (= :timed-out ret))
+      (is (< 0.1 diff 0.5) (str "deref timeout is outside of [100ms, 500ms] range: " diff "ms")))))
+
 ;;;;;;;;;;;;;
 ;; Futures ;;
 ;;;;;;;;;;;;;
@@ -1416,8 +1431,14 @@
       (is (= true (realized? fut)))))
 
   (testing "timed deref of future"
-    (let [fut (future (time/sleep 3))]
-      (is (= :timed-out (deref fut 0.01 :timed-out)))
+    (let [fut (future (time/sleep 3))
+          start (time/perf-counter)
+          ;; block for 100ms
+          ret (deref fut 100 :timed-out)
+          end (time/perf-counter)
+          diff (- end start)]
+      (is (= :timed-out ret))
+      (is (< 0.1 diff 0.5) (str "deref timeout outside of [100ms, 500ms] range: " diff "ms"))
       (is (= false (future-cancelled? fut)))
       (is (= false (future-done? fut)))
       ;; can't always cancel a sleep-ed Future