Cache validators recursively

[frenchy64]

In some cases, we can save a lot of work and memory by pushing in the -cached logic further into the schemas. The tests demonstrate a key use-case: a schema that is only ever used as a child.

This is the output of the final line of the test on master:

{:-validator 10, :-parser 10, :-unparser 10, :-explainer [[0] [0]]}

i.e., this PR saves 27 calls to protocol methods, each returning fresh fns.

Moving the explainer cache to the entire fn also saves recreating the fn every call.

[frenchy64]

There's something bugging me about the dynamism being lost here. For example, does this interfere with dynamically scoped registries in a way that matters?

[ikitommi]

There's something bugging me about the dynamism being lost here. For example, does this interfere with dynamically scoped registries in a way that matters?

I think it does. I would assume that I can recreate a schema validator with current bindings when calling -validator on it, no caching. Caching should be a layer, no built-it. This can cause hard to debug problems.

[frenchy64]

I would assume that I can recreate a schema validator with current bindings when calling -validator on it, no caching.

I'm not sure that holds even today, the caching of -pointer here pins ::foo to :string even before we call m/-validator.

(ns malli.registry-test
  (:require [clojure.test :refer [deftest is testing]]
            [malli.core :as m]
            [malli.registry :as mr]))

(let [registry* (atom {})
      register! (fn [t ?s] (swap! registry* assoc t ?s))
      registry (mr/composite-registry
                {:map (m/-map-schema)}
                (mr/mutable-registry registry*)
                (mr/dynamic-registry))
      _ (register! :maybe (m/-maybe-schema))
      s (binding [mr/*registry* {::foo (m/-string-schema)}]
          (m/schema [:map [:maybe [:maybe ::foo]]] {:registry registry}))]
  (binding [mr/*registry* {::foo (m/-int-schema)}]
    (is ((m/-validator s) {:maybe "sheep"}))
    (is (not ((m/-validator s) {:maybe 1})))))

EDIT: it later occurred to me this probably isn't -pointer doing the caching since :foo is a constructor via -lookup!.

[opqdonut]

We discussed this with Tommi & some Metosin guys. We don't feel good about this PR for a couple of reasons:

• Actual performance impact is undemonstrated. We think that eg. reitit stores the m/schema instances and calls m/validator on them, so it gets caching even now.
• Potentially surprising effects with dynamic registries etc.
• Creating a validator doesn't really need to be fast, what matters is that the validator itself is fast.

[opqdonut]

(rescinding my previous approval)