Fork me on GitHub
#announcements
<
2022-08-21
>
ambrosebs18:08:28

https://github.com/plumatic/schema 1.4.0 Adds https://plumatic.github.io/schema/schema.core.html#var-defprotocol for documenting and instrumenting protocol methods.

(s/defprotocol MyProtocol
    "Docstring"
    :extend-via-metadata true
    (^:always-validate method1 :- s/Int
      [this a :- s/Bool]
      [this a :- s/Any, b :- s/Str]
      "Method doc2")
    (^:never-validate method2 :- s/Int
      [this]
      "Method doc2"))

🚀 15
👍 5
🙌 3
robert-stuttaford05:08:46

so cool that this library is still going and still providing its same good value!

👆 1
👍 1
vemv09:08:23

Very cool! Curious, is it a zero-cost abstraction? That is, when checking is off, ideally this would be identical to vanilla protocol usage. With clojure.spec hacks normally there's a backing defn, which adds indirection and a cost. I've been meaning to try again for a while.

ambrosebs23:08:09

@U45T93RA6 mostly no, there is a cost. there are essentially 3 different modes: instrumentation on, instrumentation off, and leave as-is (see the docstring for the 3rd). In the first two, inlining is disabled, so direct calls will pay an extra indirection + cache lookup in those cases. In normal wrapped calls, there is an extra indirection that also passes the cache via set! to the "actual" protocol function.

ambrosebs23:08:56

perhaps it's a good idea to use the 3rd mode in prod, then it would be zero-cost (at least, that's what I intended).

ambrosebs23:08:53

you can move between mode 1 and 2, but mode 3 is permanent. so yeah, it's probably a good candidate for prod.

vemv06:08:52

Perhaps the docstring could be a little clearer since one might not immediately know/remember what inlinling is, etc. Also if this is problematic for your environment is bit of a strange wording, because disabling checking in production is something I'd do out of principles, not because of 'problems' Anyway this does look like a great defprotocol. It looks like one could do this at macroexpansion-time?

(if *assert*
  s/defprotocol
  defprotocol)
With other solutions I've seen (and written!) one can't, because the method names for declarations will be different vs. the method names for usages.

👍 1
ambrosebs20:08:38

Agreed, actually the wording assumes you'd only want this if Clojure's implementation details change and breaks instrumentation. I didn't really consider you could use it for perf reasons until this conversation.

ambrosebs20:08:54

> With other solutions I've seen (and written!) one can't, because the method names for declarations will be different vs. the method names for usages. Yes, I studied your impl in speced.def while designing this, and s/defprotocol does not rename methods. the same name is used to extend and invoke.

vemv10:08:28

Not sure if there was anything worth studying :) Is the gist of your solution the following? It works, would love to port it to speced.def. Please LMK if there's some missing piece

(clojure.core/defprotocol P
  (do-it [this]))

(def sync! (fn [^clojure.lang.AFunction outer-mth
                ^clojure.lang.AFunction inner-mth]
             (when-not (identical? (.__methodImplCache outer-mth)
                                   (.__methodImplCache inner-mth))
               ;; lock to prevent outdated outer caches from overwriting newer inner caches
               (locking inner-mth
                 (set! (.__methodImplCache inner-mth)
                       ;; vv WARNING: must be calculated within protected area
                       (.__methodImplCache outer-mth)
                       ;; ^^ WARNING: must be calculated within protected area
                       )))))

(let [inner-method do-it
      instrument! (fn [f] ;; the value of @#'do-it
                    (fn [x] ;; a fn with the same sig as do-it
                      (assert (pos? x)) ;; an example assertion, as it would be provided by users
                      ;; invoke whatever `do-it` does:
                      (f x)))
      outer-method (instrument! inner-method)]
  (alter-var-root #'P assoc-in [:method-builders #'do-it] (fn [cache]
                                                            (set! (.__methodImplCache outer-method) cache)
                                                            (sync! outer-method inner-method)
                                                            outer-method))
  (alter-meta! #'do-it assoc :inline (fn [& args]
                                       `((do ~(symbol #'do-it))
                                         ~@args)))
  (alter-var-root #'do-it (constantly outer-method)))

(extend-protocol P
  Long
  (do-it [this]
    2))
(do-it 1)
(do-it -1) ;; the assertion is triggered

(extend-protocol P
  Double
  (do-it [this]
    2.0))
(do-it 1.0)
(do-it -1.0) ;; the assertion is triggered

vemv10:08:34

(also, does that do in the alter-meta! add something?)

ambrosebs15:08:00

@U45T93RA6 yep, looks like it covers all the bases for Clojure JVM. the do is the dirty hack to stop the compiler inlining the method call: ((do do-it) ...) doesn't inline, but (do-it ...) can. these tests should fail without the do https://github.com/plumatic/schema/blob/0a699d7715de623e15ddc1dbc69ae1b634860e4f/test/cljc/schema/core_test.cljc#L1710-L1730

👀 1
ambrosebs15:08:49

I wanted to make a shared library with these ideas but I couldn't decide which functions to expose. Once we have a few impls, maybe it'll be more obvious.

ambrosebs15:08:26

there was also interest in adding this to malli.

vemv15:08:30

Thanks much :) will see if I can create a tiny ns with the core ideas

👏 1
vemv17:08:28

Besides from sync! , I think I'd only need this defn:

(defn instrument! [protocol-var
                   fns #_{#'foo ;; a var from the protocol, to be instrumented
                          (fn [f] ;; always receives a single function `f`, representing the original value of @#'foo
                            (fn [this x y z] ;; consumers must build this argv the right arities, for each method (and possibly for each arity of the method)
                              (f this x y z)))} ;; consumers must build the right calls to `f`
                   ]
  (doseq [[var-ref wrapper] fns
          :let [inner-method @var-ref
                outer-method (wrapper inner-method)]]
    (alter-var-root protocol-var assoc-in [:method-builders var-ref] (fn [cache]
                                                                       (set! (.__methodImplCache outer-method) cache)
                                                                       (sync! outer-method inner-method)
                                                                       outer-method))
    (alter-meta! var-ref assoc :inline (fn [& args]
                                         `((do ;; do: for preempting inlining
                                             ~(symbol var-ref)) ;; n.b. `symbol` shortcut not available in older clojures
                                           ~@args)))
    (alter-var-root var-ref (constantly outer-method))))
individual libraries (e.g. speced.def, malli-whatever) would be responsible for passing the right fns map. Each library likely does its own parsing anyway so it's not much of a requirement This API would make sense for me for speced.def. I just tried it now with some speced/fn s and it works - would fit for a defprotocol reimpl. Would be thankful if you release this as an isolated library. I actually have to be pedantic about copyright :)

vemv17:08:13

(Also since I use malli at work, I'd give two separate usages to it!)

ambrosebs17:08:22

Thanks @U45T93RA6! I'll spin up a lib.

🙂 1
vemv15:11:17

Hey there :) was there luck with the lib?