metosin / malli

High-performance data-driven data specification library for Clojure/Script.
Eclipse Public License 2.0
1.48k stars 209 forks source link

Should schemas memoize their derived objects? #498

Open bsless opened 3 years ago

bsless commented 3 years ago

By derived objects, I mean validator, explainer, de/encoder, etc.

What exists currently:

Let us consider the following simple case:

(def m0
  (m/schema
   [:map
    [:a int?]
    [:b int?]]))

(def m1
  (m/schema
   [:map
    [:x m0]
    [:y m0]]))

(def d
  (m/decoder m1 (mt/string-transformer)))

If we pick apart the decoder object:

Class: malli.core$_guard$fn__12379
Value: "#function[malli.core/-guard/fn--12379]"
---
Fields:
  "__methodImplCache" = nil
  "pred" = clojure.core$map_QMARK___5429@584a88a2
  "tf" = malli.core$_comp$fn__12284@186df06d

### tf ###

Class: malli.core$_comp$fn__12284
Value: "#function[malli.core/-comp/fn--12284]"
---
Fields:
  "__methodImplCache" = nil
  "f" = malli.core$_entry_transformer$fn__12430@3799a5cf
  "g" = malli.core$_entry_transformer$fn__12430@4165489c

### f ###
Class: malli.core$_entry_transformer$fn__12430
Value: "#function[malli.core/-entry-transformer/fn--12430]"
---
Fields:
  "__methodImplCache" = nil
  "k" = :x
  "t" = malli.core$_guard$fn__12379@2509485

### g ###
Class: malli.core$_entry_transformer$fn__12430
Value: "#function[malli.core/-entry-transformer/fn--12430]"
---
Fields:
  "__methodImplCache" = nil
  "k" = :y
  "t" = malli.core$_guard$fn__12379@64102195

What we see here is the transformer functions for each of the keys, x, and y are different objects, while they were derived from the same schema with the same transformer. If we memoized the decoder they would have been the same object

Why consider this option?

Especially when building big complex schemas, this can potentially improve performance when deriving decoders where multiple references to a schema exist. This translates effectively to faster startup

Another consideration is how the JVM's JIT operates: there is a limit to the JIT's code cache size. By putting more objects and method calls on a potentially hot path, we give the JIT compiler more code to compile, potentially forcing a deoptimization of another method to make room. This can contribute to overall improved performance.

How can we test this and how can we determine viability:

Criteria for acceptance / rejection: Up to you, really, I just come up with weird ideas.

ikitommi commented 3 years ago

Definetely. But not just the derived objects but the Schema objects themselves:

(= (m/schema :int) (m/schema :int))
; => false

... for any large schema system, that really hogs CPU & memory. If you happen to also use schema transformation utilities such as m/walk, the waste can grow exponentially.

A bounded cache baked into the core? Something like #236 would make it non-global.

related #299 (just closed).

bsless commented 3 years ago

Ofc, you're right, the schema itself should be cached as well. The question then boils down to: Local cache per IntoSchema/schema instance, semi-global cache per context The context based solution also solves all the issues with registries. Are the solutions mutually exclusive? The context solution requires lots of thinking and work, while the cached solution is pretty immediate. If they don't preclude each other, I would suggest work can start on a localized cache

ikitommi commented 3 years ago

separated schema creation into separate issue, with analysis #513.

ikitommi commented 2 years ago

this will be resolved in #550.

ikitommi commented 2 years ago

actually, caching decoders & encoders is not trivial: the -transformer method takes both the transformer and the options map, which both effect how the transformer works => we should cache based on all args, which would be a potential memory leak as the options can have anything.

forms, validators, explainers and parsers are identical per Schema, so easy to cache.,