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

Pull protocol #5

Open
julienfantin opened this issue Jul 24, 2017 · 0 comments
Open

Pull protocol #5

julienfantin opened this issue Jul 24, 2017 · 0 comments

Comments

@julienfantin
Copy link
Contributor

The pull api was extend by adding an options map used to parametrize database accessors in the query parser. While this allowed pull to work with ratoms as well as maps, it is not idiomatic and caused our implementation to diverge from MapGraph's.

We could simplify the pull api and bring it closer to its original implementation by defining a pull protocol:

(defprotocol Pull
  (ref? [db x])
  (get-ref [db ref]))

Pros

  • Would simplify the implementation of pull
  • Would make it easier to integrate upstream
  • Would make the api more extensible (though extending the parser is still a non-goal)

Cons

  • Would need to ship 6 implementations: hash and array maps for clj and cljs, plus ratom and atom
  • Last time I checked cljs protocols incurred a non-negligible performance overhead compared to simple functions. We should definitely run some cljs benchmarks before and after these changes.

Notes

  • Quick comparison of fn vs protocol performance in cljs
(def iterations 1e5)

(defn inc-in-map [x {:keys [inc-fn]}] (inc-fn x))

(let [options {:inc-fn inc}]
  (time (dotimes [_ iterations] (inc-in-map 1 options))))

;;  Compare extend-protocol vs. extend-type

(defprotocol IInc
  (inc-extend-protocol [_])
  (inc-extend-type [_]))

(extend-protocol IInc
  js/Number
  (inc-extend-protocol [x] (inc x)))

(extend-type js/Number
  IInc
  (inc-extend-type [x] (inc x)))


(time
 (dotimes [_ iterations]
   (inc-extend-protocol 1)))

(time
 (dotimes [_ iterations]
   (inc-extend-type 1)))

;; Running on node with optimizations :none
;; "Elapsed time: 16.626681 msecs"
;; "Elapsed time: 8.267159 msecs"
;; "Elapsed time: 9.751357 msecs"

;; Running on node with optimizations :advanced
;; "Elapsed time: 27.000000 msecs"
;; "Elapsed time: 8.000000 msecs"
;; "Elapsed time: 8.000000 msecs"
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

1 participant