taoensso / sente

Realtime web comms library for Clojure/Script
https://www.taoensso.com/sente
Eclipse Public License 1.0
1.74k stars 193 forks source link

Binary data support #398

Closed rosejn closed 2 years ago

rosejn commented 2 years ago

With these modifications I was able to implement a msgpack packer in both Clojure and Clojurescript, so we are now transmitting large binary typed arrays over the websocket channel. I'm also working on unifying clojure-msgpack and msgpack-cljs libs, and once that work is done I'm happy to also provide the packers so future Sente users can have plug and play binary data support. Since often times people want to use websockets for dashboards which can often be data heavy, I think this will be a welcome addition. Please let me know if you have any thoughts or feedback, and I'm happy to work with you on these changes if you have other ideas. Thanks for Sente!

ptaoussanis commented 2 years ago

@rosejn Hi Jeff, thanks a lot for this! Apologies for the long delay replying.

In principle very happy to add support for binary packers to Sente. Unfortunately the current PR would break compatibility between new packers and old unpackers.

I'm not opposed to that if it's completely unavoidable - but it would be nice to try and preserve backwards-compatibility if possible.

To better understand your needs, it would be helpful to see an example of what your binary IPacker implementation actually looks like. Would it be possible to share some more details?

Aiming to cut a new Sente release in the next few days, so there's an opportunity to get this change included soon if I can get your feedback before then.

Thanks!

rosejn commented 2 years ago

Hi, so I put together a cljc msgpack library that merged the old libs I had referenced. I contacted those authors too and nobody replied, so I'll probably just repackage this and publish it so others can use it.

https://github.com/rosejn/clojure-msgpack/tree/cljc

Here's the code for our current packer:


(ns mxv.server.stream.msgpack
  (:require
    [msgpack.core :as msgpack]
    [msgpack.extensions]
    [msgpack.macros :refer [extend-msgpack]]
    [taoensso.sente.interfaces :refer [IPacker]])
  (:import
    [java.nio ByteBuffer FloatBuffer ByteOrder]
    java.time.Instant))

(defn hex
  "Convert byte sequence to hex string"
  [coll]
  (let [hex [\0 \1 \2 \3 \4 \5 \6 \7 \8 \9 \a \b \c \d \e \f]]
      (letfn [(hexify-byte [b]
        (let [v (bit-and b 0xFF)]
          [(hex (bit-shift-right v 4)) (hex (bit-and v 0x0F))]))]
        (apply str (interleave
                     (mapcat hexify-byte coll)
                     (repeat " "))))))

; Add packing support for java.time.Instant
(extend-msgpack
  java.time.Instant
  100

  (pack
    [instant]
    (msgpack/pack (.toString instant)))

  (unpack
    [bytes]
    (java.time.Instant/parse (String. bytes))))

(deftype MsgPacker
  []
  IPacker
  (pack   [_ x]
    (msgpack/pack x))

  (unpack [_ s]
    (let [msg (msgpack/unpack s)]
      msg)))

(defn msgpack-packer
  []
  (MsgPacker.))

And on the client it's equally simple:

(deftype MsgPacker []
  IPacker
  (pack   [_ x] (msgpack/pack x))
  (unpack [_ s] (msgpack/unpack s)))

(defn msg-packer
  []
  (MsgPacker.))

Let me know if you need anything else. It would be great to get this support into mainline Sente so we don't need to reference my own branch in our production project anymore. Thanks!

ptaoussanis commented 2 years ago

@rosejn Thanks for the quick response Jeff!

Merged and released on Clojars as [com.taoensso/sente "1.17.0-RC2"] 👍

Please note that the merged commit includes some changes to PR, notably:

Feedback welcome - changes can still be made before final v1.17.0 release.