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

Hodgepodge of optimizations #546

Merged
merged 7 commits into from
Sep 27, 2024

Conversation

alexander-yakushev
Copy link
Contributor

@alexander-yakushev alexander-yakushev commented Sep 27, 2024

Following #545, here's a bunch of further improvements to HoneySQL efficiency. Commits are meaningful, I've included the explanations in the commit descriptions.

Benchmark I've used:

(format {:select [[[:min :recent_views.user_id] :user_id]
                  :model
                  :model_id
                  [[:max [:coalesce :d.view_count :t.view_count]] :cnt]
                  [:%max.timestamp :max_ts]]
         :group-by  [:model :model_id]
         :where     [:and
                     [:= :context "view"]
                     [:in :model #{"dashboard" "table"}]]
         :order-by  [[:max_ts :desc] [:model :desc]]
         :limit     10
         :left-join [[:report_dashboard :d]
                     [:and
                      [:= :model "dashboard"]
                      [:= :d.id :model_id]]
                     [:metabase_table :t]
                     [:and
                      [:= :model "table"]
                      [:= :t.id :model_id]]]}
        {:dialect :ansi})
Before:
Time per call: 50.31 us   Alloc per call: 121,104b
Time per call: 45.95 us   Alloc per call: 121,171b
Time per call: 43.92 us   Alloc per call: 121,104b

After:
Time per call: 35.65 us   Alloc per call: 82,392b
Time per call: 35.39 us   Alloc per call: 82,392b
Time per call: 35.25 us   Alloc per call: 82,392b

Converting Character->String involves additional allocations. Passing literal
strings avoids that.
src/honey/sql.cljc Outdated Show resolved Hide resolved
re-find sets up the regex machinery which is unnecessary here. str/includes? of
a single character is more efficient.
If the group is non-capturing, Clojure will not form match groups with
`re-groups`. We only use this regex as a predicate and don't need those.
@seancorfield seancorfield merged commit 40d9aee into seancorfield:develop Sep 27, 2024
4 checks passed
seancorfield added a commit that referenced this pull request Sep 27, 2024
Signed-off-by: Sean Corfield <sean@corfield.org>
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