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

Performance considerations #14

Closed
borkdude opened this issue May 24, 2020 · 2 comments
Closed

Performance considerations #14

borkdude opened this issue May 24, 2020 · 2 comments

Comments

@borkdude
Copy link
Contributor

borkdude commented May 24, 2020

See retrogradeorbit/bootleg#65. When pods have lots of client side code and many namespace all of this code is evaluated during load-pod, which is potentially a waste of time, since not every script is going to use all of the evaluated code.

Maybe we could defer loading of pod namespace code to the require of that namespace. This would work for sci since we have control over require. For Clojure it would be slightly more complicated. We'd probably have to patch load to take control over loading pod namespaces:

$ clj
Clojure 1.10.1
user=> (let [old-load clojure.core/load] (intern 'clojure.core 'load (fn [& paths] (doseq [path paths] (if (= "pod.foo.ns" path) (prn "load pod ns! ") (old-load path))))))
#'clojure.core/load
user=> (load "pod.foo.ns")
"load pod ns! "
nil
user=> (load "clojure.set")
nil

I did a quick experiment with this (see lazy-namespaces branches in babashka and babashka.pods).

I did have to introduce extra requires to explicitly load the namespace dependencies in the right order:

(ns mybbscript
  (:require [babashka.pods :as pods]))

(println "loading pod")
(time (pods/load-pod "bootleg"))

(println "loading utils")
(time (require '[pod.retrogradeorbit.bootleg.utils]))

(println "loading xml")
(time (require '[pod.retrogradeorbit.net.cgrand.xml]))

(println "loading enlive-html")
(time (require '[pod.retrogradeorbit.net.cgrand.enlive-html]))

(println "loading enlive")
(time (require '[pod.retrogradeorbit.bootleg.enlive :as enlive]))

(time
 (enlive/at "<p>{{here comes foo}}</p>" [:p]
            (enlive/content "foo")))
$ time ./bb /tmp/bootleg.clj
loading pod
"Elapsed time: 34.016951 msecs"
loading utils
"Elapsed time: 1.771767 msecs"
loading xml
"Elapsed time: 0.507533 msecs"
loading enlive-html
"Elapsed time: 34.126741 msecs"
loading enlive
"Elapsed time: 30.818078 msecs"
"Elapsed time: 16.301291 msecs"
"<p>foo</p>"
./bb /tmp/bootleg.clj   0.08s  user 0.04s system 78% cpu 0.153 total

The net win compared to loading all the namespaces during startup doesn't seem to be so big here since this example still requires the namespaces that are macro heavy:

$ time /usr/local/bin/bb /tmp/bootleg.clj
loading pod
"Elapsed time: 115.301959 msecs"
loading utils
"Elapsed time: 0.020333 msecs"
loading xml
"Elapsed time: 0.013172 msecs"
loading enlive-html
"Elapsed time: 0.011473 msecs"
loading enlive
"Elapsed time: 0.012528 msecs"
"Elapsed time: 15.561942 msecs"
"<p>foo</p>"
/usr/local/bin/bb /tmp/bootleg.clj   0.10s  user 0.03s system 78% cpu 0.165 total

However, this example does much better:

(ns mybbscript
  (:require [babashka.pods :as pods]))
(println "loading pod")
(time (pods/load-pod "bootleg")) ;; 36ms
(require '[pod.retrogradeorbit.bootleg.markdown :refer [markdown]]) ;; 0.09ms
(markdown "# Title" :data :html) ;;=> "<h1>Title</h1>"

Alternatives:

Custom require for pods: (pods/require 'retrogradeorbit.bootleg.enlive :as enlive)
Pods only declare their namespaces in describe, but not the associated vars. With pods/require (syntactically the same as require) pod namespaces get loaded.
This would be easier to implement for both sci and Clojure. We could still support the old way of loading pods. Syntactically it would be less appealing and would require configuration for clj-kondo.

@retrogradeorbit
Copy link
Member

I think loading namespace code definitions on require is a great mitigation as quite often different namespaces can be for different use cases. For example I have lots of bootleg code that uses enlive but not hickory, and other that use hickory and not enlive. So makes sense to only load that which is being used.

@borkdude
Copy link
Contributor Author

See #15 for a concrete implementation proposal for splitting up per namespace.

@borkdude borkdude closed this as completed Jun 8, 2020
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