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

Issue #381 Reduce the number of Eval calls #396

Merged
merged 15 commits into from
Aug 13, 2018
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ This is a history of changes to clara-rules.

### 0.19.0-SNAPSHOT
* Remove a warning about `qualified-keyword?` being replaced when using Clojure 1.9.
* Batch evaluation of node expressions for better compilation performance. See [issue 381](https://github.com/cerner/clara-rules/issues/381)

### 0.18.0
* Remove unnecessary memory operations from ProductionNode to optimize performance. Remove :rule-matches in session inspection that did not cause logical insertions and add a new optional feature to return all rule matches, regardless of what their RHS did or whether they were retracted. Add a new listener and tracing method fire-activation!. These changes are moderately non-passive with respect to listening, tracing, and session inspection but are otherwise passive. See [issue 386](https://github.com/cerner/clara-rules/issues/386) for details.
Expand Down
11 changes: 9 additions & 2 deletions project.clj
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,15 @@
;; perhaps because Leiningen is using this as uneval'ed code.
;; For now just duplicate the line.
:test-selectors {:default (complement (fn [x]
(some->> x :ns ns-name str (re-matches #"^clara\.generative.*"))))
:generative (fn [x] (some->> x :ns ns-name str (re-matches #"^clara\.generative.*")))}
(let [blacklisted-packages #{"generative" "performance"}
patterns (into []
(comp
(map #(str "^clara\\." % ".*"))
(interpose "|"))
blacklisted-packages)]
(some->> x :ns ns-name str (re-matches (re-pattern (apply str patterns)))))))
:generative (fn [x] (some->> x :ns ns-name str (re-matches #"^clara\.generative.*")))
:performance (fn [x] (some->> x :ns ns-name str (re-matches #"^clara\.performance.*")))}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Having this live in the main Clara project seems reasonable to me, but there is an existing (old) clara-benchmark project. FYI @rbrush in case you have suggestions, objections, etc.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm in favor of including this in the main Clara project. The test selectors are a nice solution to performance concerns here.


:scm {:name "git"
:url "https://github.com/cerner/clara-rules"}
Expand Down
12 changes: 8 additions & 4 deletions src/main/clojure/clara/macros.clj
Original file line number Diff line number Diff line change
Expand Up @@ -196,10 +196,11 @@
(sc/defn ^:always-validate compile-alpha-nodes
[alpha-nodes :- [schema/AlphaNode]]
(vec
(for [{:keys [condition beta-children env]} alpha-nodes
(for [{:keys [id condition beta-children env]} alpha-nodes
:let [{:keys [type constraints fact-binding args]} condition]]

{:type (com/effective-type type)
{:id id
:type (com/effective-type type)
:alpha-fn (com/compile-condition type (first args) constraints fact-binding env)
:children (vec beta-children)})))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems odd to me that we build a map here and then use it to construct alpha nodes later in assemble-session while building the beta nodes directly in this namespace, but changing that is probably out of scope here. Maybe there's a reason for this that I'm not remembering offhand.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree that is a little weird the only thing i think that is forcing this is, the access to the compiled beta nodes. At this point the beta-children are just ids of the nodes that will be constructed later.


Expand All @@ -209,11 +210,14 @@
;;; as a ClojureScript DSL may call productions->session-assembly-form if that DSL
;;; has its own mechanism for grouping rules which is different than the clara DSL.
(com/validate-names-unique productions)
(let [beta-graph (com/to-beta-graph productions)
(let [id-counter (atom 0)
create-id-fn (fn [] (swap! id-counter inc))

beta-graph (com/to-beta-graph productions create-id-fn)
;; Compile the children of the logical root condition.
beta-network (gen-beta-network (get-in beta-graph [:forward-edges 0]) beta-graph #{})

alpha-graph (com/to-alpha-graph beta-graph)
alpha-graph (com/to-alpha-graph beta-graph create-id-fn)
alpha-nodes (compile-alpha-nodes alpha-graph)]

`(let [beta-network# ~beta-network
Expand Down
7 changes: 5 additions & 2 deletions src/main/clojure/clara/rules.cljc
Original file line number Diff line number Diff line change
Expand Up @@ -210,9 +210,9 @@
[(:id node) node]))

;; type, alpha node tuples.
alpha-nodes (for [{:keys [type alpha-fn children env]} alpha-fns
alpha-nodes (for [{:keys [id type alpha-fn children env]} alpha-fns
:let [beta-children (map id-to-node children)]]
[type (eng/->AlphaNode env beta-children alpha-fn)])
[type (eng/->AlphaNode id env beta-children alpha-fn)])

;; Merge the alpha nodes into a multi-map
alpha-map (reduce
Expand Down Expand Up @@ -329,6 +329,9 @@
It defaults to checking the :salience property, or 0 if none exists.
* :activation-group-sort-fn, a comparator function used to sort the values returned by the above :activation-group-fn.
Defaults to >, so rules with a higher salience are executed first.
* :forms-per-eval - The maximum number of expressions that will be evaluated per call to eval.
Larger batch sizes should see better performance compared to smaller batch sizes. (Only applicable to Clojure)
Defaults to 5000, see clara.rules.compiler/forms-per-eval-default for more information.

This is not supported in ClojureScript, since it requires eval to dynamically build a session. ClojureScript
users must use pre-defined rule sessions using defsession."
Expand Down
Loading