Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Better performance for anonymous schematized functions, via lazy checker creation #305

Merged
merged 1 commit into from
Dec 4, 2015

Conversation

w01fe
Copy link
Member

@w01fe w01fe commented Nov 22, 2015

Fixes plumatic/plumbing#92

Unfortunately I don't know of a way to hoist the schema checker creation for anonymous functions, so it was happening on every creation instance. This PR adds a delay around checker creation (and makes another small optimization), making it about 20 times faster to create anonymous functions (or fnks) than previously. Should also speed up load time for schematized codebases slightly.

w01fe added a commit that referenced this pull request Dec 4, 2015
Better performance for anonymous schematized functions, via lazy checker creation
@w01fe w01fe merged commit 7413a5c into master Dec 4, 2015
@w01fe w01fe deleted the faster-anonymous-fns branch December 4, 2015 10:49
@bfabry
Copy link
Contributor

bfabry commented Dec 15, 2015

so this shifts the time creating the checkers from fn-creation-time to fn-first-execution-time right? what's the situation where that's useful? asking because attaching mutable things like Delay's to a fn makes them hard to serialize, as in #308

@w01fe
Copy link
Member Author

w01fe commented Dec 15, 2015

The main reason for the change is that if you use s/fn (or plumbing.core/fnk), we have to create the checker each time we create an anonymous function (since there's no way I know of to lift it outside the body of the containing form to make it happen just once). This makes these expressions 20x slower than they need to be, even when validation is not being used. If you can think of an alternative that has the same effect, we'd be happy to consider it -- sorry for the difficulty.

@bfabry
Copy link
Contributor

bfabry commented Dec 16, 2015

Now that I look at it more closely, I'm actually quite confused by the delay, it seems like it is created and derefed at the same time, and so shouldn't make any difference to performance at all.

(def a-sym `(do (println "thunk") 5))
=> #'user/a-sym
`[~a-sym]
=> [(do (clojure.core/println "thunk") 5)]
(def a-sym `(delay (do (println "thunk") 5)))
=> #'user/a-sym
`[@~a-sym]
=> [(clojure.core/deref (clojure.core/delay (do (clojure.core/println "thunk") 5)))]

@w01fe
Copy link
Member Author

w01fe commented Dec 16, 2015

That's not what's going on though -- the delay is bound outside the fn, and deref'd inside.

user> (pprint (macroexpand '(s/fn [])))
(let*
 [ufv__
  schema.utils/use-fn-validation
  output-schema9819
  schema.core/Any
  input-schema9820
  []
  input-checker9821
  (clojure.core/delay (schema.core/checker input-schema9820))
  output-checker9822
  (clojure.core/delay (schema.core/checker output-schema9819))]
 (schema.core/schematize-fn
  (clojure.core/fn
   fn9818
   ([]
    (clojure.core/let
     [validate__8342__auto__ (.get_cell ufv__)]
     (clojure.core/when
      validate__8342__auto__
      (clojure.core/let
       [args__8343__auto__ []]
       (clojure.core/when-let
        [error__8344__auto__ (@input-checker9821 args__8343__auto__)]
        (schema.macros/error!
         (schema.utils/format*
          "Input to %s does not match schema: %s"
          'fn9818
          (clojure.core/pr-str error__8344__auto__))
         {:schema input-schema9820,
          :value args__8343__auto__,
          :error error__8344__auto__}))))
     (clojure.core/let
      [o__8345__auto__ (clojure.core/loop [])]
      (clojure.core/when
       validate__8342__auto__
       (clojure.core/when-let
        [error__8344__auto__ (@output-checker9822 o__8345__auto__)]
        (schema.macros/error!
         (schema.utils/format*
          "Output of %s does not match schema: %s"
          'fn9818
          (clojure.core/pr-str error__8344__auto__))
         {:schema output-schema9819,
          :value o__8345__auto__,
          :error error__8344__auto__})))
      o__8345__auto__))))
  (schema.core/->FnSchema output-schema9819 [input-schema9820])))

@bfabry
Copy link
Contributor

bfabry commented Dec 16, 2015

ah, I was reading the value of :more-bindings as a let binding form, when it's actually creating a vector used elsewhere. will have to dig deeper, cheers

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants