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

[fix](query) improve performance total count using cache #57

Merged
merged 1 commit into from
May 31, 2020

Conversation

areski
Copy link
Collaborator

@areski areski commented May 28, 2020

count might seem innocent, but it quickly can get very painful if you are working with millions of records.

In this PR, we first change count(r.d) by count(*), the second is slightly more performant, it might be counter intuitive but an explanation is giving here why https://www.cybertec-postgresql.com/en/postgresql-count-made-fast/

There is a tiny of refactoring to have a single source for counting through total_count

Finally, we introduce a caching, this is definitely arguable.
I can understand many good reason for trying to keep the deps as low as possible in kaffy.

We could certainly implement our own cache: https://thoughtbot.com/blog/make-phoenix-even-faster-with-a-genserver-backed-key-value-store if really using cachex.
Having say that cachex seems to be a very well tested packages.

Personally I have been using https://github.com/melpon/memoize heavily in a backend service production but it seems that cachex might be a better fit for phoenix and it's certainly more active.

@aesmail
Copy link
Owner

aesmail commented May 28, 2020

Your point is very valid. This definitely needs a fix. Currently with some resources, I have to wait a few seconds before the page could render which is not fun.

We could use cachex, however I feel very uneasy adding it (which adds its own dependencies) just to temporarily hold simple numbers. As you mentioned, this could easily be implemented in a few lines with GenServer or even with ETS which are both built into elixir.

It's almost as easy as:

:ets.new(:total_count, [:named_table])
:ets.insert(:total_count, {"schema_count", 3_000_000})
count = :ets.lookup(:total_count, "schema_count")
# count = [{"schema_count", 3000000}]

This needs just a couple more lines to check if the :total_count table already exists with an entry for "reseting" the values after a few minutes.

Either way, this problem needs to be fixed. And I recommend using our own extremely simple solutions as oppose to using an external dependency.

@aesmail aesmail merged commit f7c25f3 into aesmail:master May 31, 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

Successfully merging this pull request may close these issues.

2 participants