Opened 10 years ago

Closed 10 years ago

#2591 closed enhancement (fixed)

rails fragment caching for diary list

Reported by: amm Owned by: Tom Hughes
Priority: minor Milestone:
Component: website Version:
Keywords: Cc:

Description

Some webpages on OSM don't load as fast as would be ideal. One of these is the diary page. With typical load times of between 1 - 3 seconds, the time is by no means terrible, but still noticeable as a lag and so would be nice to reduce. Cacheing is one option and should help here as the the diary page is well suited for rails fragment caching. The diary list page changes relatively seldomly, and there aren't too many dynamical alternatives.

I have therefore attached a patch implementing a simple fragment caching for diary_entry/list.html.erb as a proof of concept to see if fragment caching can reduce load times and how much overhead it adds. So I would mainly be interested in comments to hear your opinion on this matter and if you think it is the right approach to improve the felt responsiveness of the website. It currently only does caching for the diary page, as apart from the GPX trace page, the diary page is the only one that has load times of over a second and it is the easiest page to try out caching.

To try and see how much potential this patch has, I have tried to do some benchmarks. All numbers were generated with apache-bench with 10 repetitions and a concurrency of 1.

First off, looking at the current numbers of how long it takes to load the diary page:

ab -n 10 -c1 http://www.openstreetmap.org/diary Connection Times (ms)

min mean[+/-sd] median max

Connect: 38 57 20.1 59 100 Processing: 1219 1733 364.3 1790 2522 Waiting: 1078 1532 355.8 1605 2300 Total: 1264 1789 363.6 1870 2560

Those numbers are obviously somewhat variable depending on how busy the server is, but the minimum time is mostly stable. To see a bit better how how much of that time is due to general network, queueing and rails delay the equivalent call to a very simple page, the login page

ab -n 10 -c1 http://www.openstreetmap.org/login

Connection Times (ms)

min mean[+/-sd] median max

Connect: 34 42 6.8 41 55 Processing: 51 66 9.5 68 79 Waiting: 51 65 9.1 67 79 Total: 86 108 13.3 108 131

Now looking at the effect of caching the diary page on my local testbed (passenger).

The first run is with caching turned off

Connection Times (ms)

min mean[+/-sd] median max

Connect: 0 0 0.0 0 0 Processing: 293 298 7.1 297 317 Waiting: 293 298 7.1 297 317 Total: 293 299 7.1 297 317

And now with caching turned on, it is

Connection Times (ms)

min mean[+/-sd] median max

Connect: 0 0 0.0 0 0 Processing: 22 55 70.4 23 235 Waiting: 22 55 70.4 23 235 Total: 22 56 70.4 23 235

So caching seems to decrease the processing time by close to ten fold for the diary list page. Given that the original rendering times on the OSM server are about 3 times larger (presumably due to the more complex real diary entries than my few test entries) I would guess the effect is even larger on the OSM servers.

But as said at the top, this is mostly a RFC at the moment and as a basis to think about how best to reduce load times of the diary page.

Attachments (3)

diary_cache.patch (8.0 KB) - added by amm 10 years ago.
A test implementation of fragment caching for the diary list page
diary_cache.2.patch (3.6 KB) - added by Tom Hughes 10 years ago.
This is my current patch
diary_cache.3.patch (5.1 KB) - added by Tom Hughes 10 years ago.
Here's my latest patch

Download all attachments as: .zip

Change History (18)

Changed 10 years ago by amm

Attachment: diary_cache.patch added

A test implementation of fragment caching for the diary list page

comment:1 Changed 10 years ago by Tom Hughes

Is it not possible to do it in a cleaner way than that? I'd have to get the books out to check but I thought caching was supposed to be more or less automatic in rails but that patch all looks horribly invasive and manual at a first glance.

I'll also need to know more about the possible issues that might arise in a production environment - where is the cache held for example? How is it's size controlled - do we need to run some sort of cleanup job from cron? What (if anything) are the coherency issues when multiple processes and/or servers are involved in serving rails requests?

comment:2 Changed 10 years ago by Tom Hughes

I think you've missed out the sweeper code from your patch?

comment:3 Changed 10 years ago by Tom Hughes

Having looked at it some more, I suspect action caching is more appropriate for what your doing here. So for the diary view method a simple declaration like this:

  caches_action :view, :cache_path => { :locale => I18n.locale }, :layout => :false
}}

should do the job. Obviously a sweeper is still needed.

It might be worth adding fragment caching for some unchanging bits of the layout though, or for individual entries in the diary list so the whole list doesn't have to be rerendered when a new entry is added.

comment:4 Changed 10 years ago by Tom Hughes

Having looked at it some more, I suspect action caching is more appropriate for what your doing here. So for the diary view method a simple declaration like this:

  caches_action :view, :cache_path => { :locale => I18n.locale }, :layout => :false

should do the job. Obviously a sweeper is still needed.

It might be worth adding fragment caching for some unchanging bits of the layout though, or for individual entries in the diary list so the whole list doesn't have to be rerendered when a new entry is added.

comment:5 Changed 10 years ago by amm

Thanks for having a look at the patch.

I chose fragment caching rather than action caching, as the page is not identical when a user is logged in or not and I thought action caching always caches the entire rendered page, so it sounded like it would be unsuitable for those dynamic parts. I didn't know about the layout option though, which sounds like it might do what is needed to allow for action caching, but I haven't yet understood what exactly his option does and haven't yet tried it out. How does it know though which parts are dynamic and can't be cached and which parts can, other than by explicitly telling it in code in the view and controller?

The patch indeed needs sweepers to expire the caches whenever a new entry or comment is added, but they should already be included. If you look at the patch in the "original format" the new file "model/diary_sweeer.rb" is included, but for some strange reason, the trac diff viewer doesn't show that part.

The fragments get cached in the fragment cache, for which there are 4 different backends available in rails.

1) Memory backend 2) Filesystem backend 3) Memcached backend 4) distributed rails backend.

The memory backend is per process and therefore not so good when having multiple rails daemons. The filesystem backend is imho the easiest and best option. It needs no extra setting up and works correctly accross multiple daemons on a single machine and can be made to work across multiple machines too. The memcached backend has the problem of not being able to expire multiple fragments via regexp expiration, which is necessary for the paginated caching. The distributed rails backend might also be a solution, especially if we should scale web serving across multiple machines, but I haven't yet tried that out to see how it works.

Fragment caching is probably possible and sensible for some other parts too, but the diary list page (not the diary view page, which I only included as it was in the same controller) seemed by far the most worth while target for the moment to start experimenting with caching.

I'll need to have a closer look at the action caching with layout => false

comment:6 Changed 10 years ago by amm

I am very much still learning about the basics of rails, and hadn't really come across the concept of layouts at all yet. It sounds like action caching without layouts would take care of a good portion of the dynamic parts of users being logged in. But the dynamic part of only having the option to add a comment in the "diary_entry/view.html.erb" and the option to add a new diary entry in the "diary_entry/list.html.erb" still depend on <% if @user %> in the actual view. But that boolean variable might be handled by a more complex cache_path including the user variable too. So it might still be possible to use action caching and not need any extra code at all other than sweepers and the caches_action statement.

comment:7 Changed 10 years ago by amm

Looking at the benchmark numbers a bit more, and particularly the difference between the response time of way over a second from osm.org vs. 300ms from my local testbed, makes me wonder how much of that second is rendering time and how much is database query time. My suspicion is that a large portion might be DB query time, as I don't have anywhere close as many comments in my testbed as in the main DB, whereas the complexity of the rendered page with only 20 diary entires is probably comparable.

Caching would obviously reduce both rendering time and db query time and thus would be preferable, but if the db query time can be reduced, that would still help every time the page isn't yet cached.

I presume, the main DB query performed in the diary list controller is the "SELECT ... WHERE ("diary_entries"."visible" = 't' AND "users"."visible" = 't') ORDER BY created_at DESC LIMIT 20 OFFSET 0". Currently there isn't an index on created_at, which presumably means the query needs to do a full seq_scan on diary_entry plus a sort over all visible diary entries. Adding an index on created_at would allow to do a filtered idx_scan without needing an extra sort. Both of these seem to be confirmed by explain analyze on my limited number of entries in my test db, but as the (db) setup is so different to osm.org I don't know how much of those "benchmarks" can be transfered.

comment:8 in reply to:  5 Changed 10 years ago by Tom Hughes

Replying to amm:

The patch indeed needs sweepers to expire the caches whenever a new entry or comment is added, but they should already be included. If you look at the patch in the "original format" the new file "model/diary_sweeer.rb" is included, but for some strange reason, the trac diff viewer doesn't show that part.

The diff viewer won't show it because it's a new file. I had made the same mistake in that I applied the patch and then used svn diff forgetting that the new file wouldn't have been added to svn so wouldn't show up.

comment:9 in reply to:  6 Changed 10 years ago by Tom Hughes

Replying to amm:

I am very much still learning about the basics of rails, and hadn't really come across the concept of layouts at all yet. It sounds like action caching without layouts would take care of a good portion of the dynamic parts of users being logged in. But the dynamic part of only having the option to add a comment in the "diary_entry/view.html.erb" and the option to add a new diary entry in the "diary_entry/list.html.erb" still depend on <% if @user %> in the actual view. But that boolean variable might be handled by a more complex cache_path including the user variable too. So it might still be possible to use action caching and not need any extra code at all other than sweepers and the caches_action statement.

Yes I realised that view sometimes vary if you are logged in without actually being dependent on who you are logged in as, so I added a logged in flag to the cache entries to cope with that. I'll attach my patch in a minute.

comment:10 in reply to:  7 Changed 10 years ago by Tom Hughes

Replying to amm:

I presume, the main DB query performed in the diary list controller is the "SELECT ... WHERE ("diary_entries"."visible" = 't' AND "users"."visible" = 't') ORDER BY created_at DESC LIMIT 20 OFFSET 0". Currently there isn't an index on created_at, which presumably means the query needs to do a full seq_scan on diary_entry plus a sort over all visible diary entries. Adding an index on created_at would allow to do a filtered idx_scan without needing an extra sort. Both of these seem to be confirmed by explain analyze on my limited number of entries in my test db, but as the (db) setup is so different to osm.org I don't know how much of those "benchmarks" can be transfered.

I think we had probably never bothered with that index because the number of diary entries was so small it didn't really matter but you're right that it would be helpful now so I've added it (well three versions of it in fact) and typical DB times for the first page of diary entries have dropped from 2-300ms to about 5ms or so.

Changed 10 years ago by Tom Hughes

Attachment: diary_cache.2.patch added

This is my current patch

comment:11 Changed 10 years ago by amm

Your patch definitely looks more elegant than mine, so thanks for correcting it.

However, I have noticed two issues with your version.

1) The expiration of fragments doesn't work correctly here, as the cache fragment is stored as "views/localhost.3000/diary.cache" but the regexp checks "localhost:3000/diary.*" notice the difference between "localhost.3000" and "localhost:3000" I suspect it works correctly if it is run on default port 80, but I suspect with all the daemons on the main page run on different ports, it would probably be preferable to drop the port number as part of the key altogether.

2) The page http://localhost:3000/user/test1/diary/ is different depending on if you log in as user test1 or any other user. So application_controller.caches_action needs an additional cache key for "self" or "other_users" pages.

Otherwise it seems to work fine and correctly caches and expires the user diary pages

comment:12 in reply to:  11 Changed 10 years ago by Tom Hughes

Replying to amm:

1) The expiration of fragments doesn't work correctly here, as the cache fragment is stored as "views/localhost.3000/diary.cache" but the regexp checks "localhost:3000/diary.*" notice the difference between "localhost.3000" and "localhost:3000" I suspect it works correctly if it is run on default port 80, but I suspect with all the daemons on the main page run on different ports, it would probably be preferable to drop the port number as part of the key altogether.

This is arguably a bug in rails really, in that the translation of ':' to '.' is hidden inside the file based cache backend which means that non-regexp expiry respects it but regexp expiry doesn't.

I've worked round it in my tree now anyway.

2) The page http://localhost:3000/user/test1/diary/ is different depending on if you log in as user test1 or any other user. So application_controller.caches_action needs an additional cache key for "self" or "other_users" pages.

I think the best thing will be to replace the logged_in flag with a tristate parameter - logged_out/logged_in_as_somebody_else/logged_in_as_this_user.

comment:13 Changed 10 years ago by amm

1) That does seem like a bug in rails. Particularly, as I guess it means it works for some caching backends, but not for all. But looking for a workaround is probably the easiest, so great that you have found one and implemented it

2) I too think that the tristate parameter would be best and covers all of the conditions I have seen in the diary pages and probably also in most of the other pages that can be reasonably dealt with via action caching.

So I think with those two fixes the patch should work. If you want I can test the new patch again, or I would also be fine with it if you are happy with it.

Changed 10 years ago by Tom Hughes

Attachment: diary_cache.3.patch added

Here's my latest patch

comment:14 Changed 10 years ago by amm

With the issues now solved we discusses yesterday on IRC, I don't see any more problems and from my side, I think the patch is ok.

I have also tested the patch in a single server multiple daemons environment on both a mongrel and thin "cluster" in production mode and didn't see any further problems so hopefully there won't be any problems when moving over to lighttpd and fastcgi either.

comment:15 Changed 10 years ago by amm

Resolution: fixed
Status: newclosed

I'll close this ticket, as it seems to be working fine now on the main site. Thanks

Note: See TracTickets for help on using tickets.