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

s/fn validation has extra overhead #450

Open
frenchy64 opened this issue Sep 30, 2022 · 3 comments
Open

s/fn validation has extra overhead #450

frenchy64 opened this issue Sep 30, 2022 · 3 comments

Comments

@frenchy64
Copy link
Contributor

The (@~input-checker-sym args#) in the input checker code could be faster in the most common case without a rest argument and be more like (@~input-checker-sym ~@bind-syms).

(when-let [error# (@~input-checker-sym args#)]
  (error! (utils/format* "Input to %s does not match schema: \n\n\t \033[0;33m  %s \033[0m \n\n"
                         '~fn-name (pr-str error#))
          {:schema ~input-schema-sym :value args# :error error#})))))
@frenchy64
Copy link
Contributor Author

frenchy64 commented Sep 30, 2022

Perhaps a new macro called something like schema.core/arguments-checker could expand like this:

(arguments-checker 'my-fn-name ([a :- s/Int] [b :- s/Int, c :- s/Num] ))
;=>
(let [a_chk (checker s/Int)
      b_chk (checker s/Int)
      c_chk (checker s/Num)
      input_schema1 [(s/one a_chk 'a)]
      input_schema2 [(s/one a_chk 'b)
                     (s/one a_chk 'c)]]
  (fn
    ([a]
     (let [a_err (a_chk a)]
       (when a_err
         (let [error [(list 'named a_err a)]]
           (error! (utils/format* "Input to %s does not match schema: \n\n\t \033[0;33m  %s \033[0m \n\n"
                                  'my-fn-name (pr-str error))
                   {:schema input-schema1 :value [a] :error error})))))
    ([b c]
     (let [b_err (b_chk b)
           c_err (c_chk c)]
       (when (or b_err c_err)
         (let [error (mapv (fn [[n err]]
                             (when err
                               (list 'named err n)))
                           [[b b_err] [c c_err]])]
           (error! (utils/format* "Input to %s does not match schema: \n\n\t \033[0;33m  %s \033[0m \n\n"
                                  'my-fn-name (pr-str error))
                   {:schema input-schema2 :value [b c] :error error})))))))

@w01fe
Copy link
Member

w01fe commented Oct 1, 2022

Interesting! A few questions:

  • How significant is the overhead of the list compared to the typical overhead of checking itself?
  • Would this require a lot of code duplication, or do you think we can just refactor the existing checks?
  • How will this tile with fn-validator ?

@frenchy64
Copy link
Contributor Author

How significant is the overhead of the list compared to the typical overhead of checking itself?

Haven't done any perf tests, just a hunch for now. I happened to be reading the schema.macros implementation with my performance hat on. I'll try some tests with the prototype.

Would this require a lot of code duplication, or do you think we can just refactor the existing checks?

I think it could replace the existing checks if it works out in practice. It may be a challenge to manage method size, I tried to factor out as much code as I could into defn's in my prototype.

How will this tile with fn-validator?

Nothing needs to change there I think.

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

No branches or pull requests

2 participants