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

Speed improvements for /admin/pages/ #921

Closed
wants to merge 3 commits into from

Conversation

jorrizza
Copy link
Contributor

These changes try to make /admin/pages/ render a lot faster.
It works, albeit not nearly fast enough for > 2000 pages. It's still useful for smaller sites.

A true solution would be to render the page asynchronously in batches or something.

These changes try to make /admin/pages/ render a lot faster.
It works, albeit not nearly fast enough for > 2000 pages.
@TeatroIO
Copy link

I've prepared a stage to preview changes. Open stage or view logs.

@mtomov
Copy link
Contributor

mtomov commented Jan 25, 2016

👍 I think this is great and it would be great to have this merged (not sure if needs a bit of code style cleanup).

I've profiled the production /pages with rack mini profiler and with only about 20-30 pages, the picture is not bells & roses:

screenshot from 2016-01-25 22 52 09


You're saying that this is still not enough, but where is the holdup? If you've eliminated the partial rendering and the SQL, what's there left, which is slowing it down. You might give the MiniProfiler a try (if you haven't done so already), more specifically the Flamegraphs.

@jorrizza
Copy link
Contributor Author

I've been playing with MiniProfiler and the flamegraphs. The rough conclusion; the next major bottleneck is the Rails router (_path functions etc.). Removing calls to those is another ~ 3x speed up. We can't do that without breaking functionality. The rest is just plumbing needed for rights management and other very useful functions. I think this page is just asking way too much of Rails. Further optimising away calls to ActiveRecord for the per-user folding stuff will only (at max) give a 9% speed boost.

While I agree with you maintainability suffers a lot by shifting rendering of this page to the client; usability will be much better. Right now, even with these optimisations, Alchemy is not usable for sites with more than about 500 pages. Depending on your infrastructure of course.

Results for 469 pages on an Intel i7-5557U, Linux 4.3.0, Ruby 2.1.5 and Unicorn 5.0.1:

Profiling results

Flamegraph

@jorrizza
Copy link
Contributor Author

With the query optimisation just now, still not much better:
Not much better

@@ -1,3 +1,4 @@
# coding: utf-8
Copy link
Member

Choose a reason for hiding this comment

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

This is not necessary, since Alchemy is Ruby >= 2 and utf-8 is default encoding

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That was added automatically by Emacs. Feel free to remove it, although in my opinion it's not a bad idea to keep it there. Some editors don't detect file encoding automatically.

@tvdeyen
Copy link
Member

tvdeyen commented Feb 4, 2016

Closing in favor of #923

@tvdeyen tvdeyen closed this Feb 4, 2016
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.

4 participants