search

plumatic / schema

Clojure(Script) library for declarative data description and validation
Other
2.4k stars 256 forks source link

Fix `s/fn` performance issues in Clojure #430

Closed frenchy64 closed 2 years ago

frenchy64 commented 2 years ago

While trying to implement s/defprotocol, I needed the following invariant to hold in Clojure to access __methodImplCache:

(let [f (s/fn this [] this)]
  (identical? f (f)))

The fix I chose was to use ^{:schema schema} (fn ...) instead of (with-meta (fn ...) {:schema schema}). I believe this also fixed all the documented performance caveats for s/fn in Clojure as it means wrappers are eliminated.

The perf characteristics for CLJS seem unchanged (still uses the wrapper). I worked around CLJS-968 by placing the function evaluation in non-tail position. I decided working around this issue was better than using macros/if-cljs (which I treat as a last resort).

I also moved a test, please ignore whitespace changes when reviewing.

frenchy64 commented 2 years ago

There was an oversight in lein all that meant the latest cljs was never tested. I needed to drop Clojure 1.7 from the test suite because CLJS doesn't work with it (but we only support Clojure 1.8 and later anyway).