hhucn / adhocracy.hhu_theme

adhocracy theme for normsetzung.cs.uni-duesseldorf.de
10 stars 7 forks source link

Run performance tests #401

Closed phihag closed 11 years ago

phihag commented 11 years ago
phihag commented 11 years ago

At least in development, disabling Routes.Mapper.always_scan speeds up adhocracy by a factor of 9! (That's with an SSD and ample RAM)

We should pay more attention to file I/O!

phihag commented 11 years ago

Unfortunately this option is only set in debug mode, but it's useless anyways.

nidico commented 11 years ago

Very good! Are you using the locally installed funkload to perform the tests?

skoenen commented 11 years ago

I noticed some little peaks in disk write on my develop machine, regarding the request log. In normal operation the complete parse and write down took 20 ~ 30 ms, but approximately all 4th or 5th execution it took 300 ~ 800 ms. (mostly on site reload, what could be the cause for this [browser and adhocracy on same machine])

phihag commented 11 years ago

@nidico No, at first I'm dvelving into the overhead of single requests, and funkload would be overkill for that, so I'm simply using ab.

Interestingly, the request log doesn't seem to have any measurable effect. I get about 50 requests / s to /stats/on_page, and enabling or disabling the requestlog doesn't effect any change (to within experimental error). However, querying any page (even if it's not actually rendering anything) in an instance lets the performance drop down to about 20 requests / s.

phihag commented 11 years ago

Dvelving into more internals has brought some interesting results:

nidico commented 11 years ago

Database queries are fast, both for sqlite(!) and MySQL.

Do you test with content in the database?

Database query construction is extremely slow, especially when an instance is searched.

Do you mean Instance.find()? This doesn't contain a join.

numerous (I see three, but there seem to be actually four) calls to h.feedback.get_something

We should cache this stuff (here: h.feedback.get_feedback_instance) locally within a request. I'm unsure how this is be best done AFAIK threads are reused by subsequent requests, so we can't simply store stuff within globals or in the module namespace. A simple solution would be to create a ThreadLocal dict in a middleware and empty it on return. This could then be used manually or through a cache_local decorator. I wonder if there isn't a standard way to do this kind of thing in WSGI apps.

phihag commented 11 years ago

Do you test with content in the database?

Sure, with initial content and our imports. For the frontpage, I don't think it matters how much content one has, does it? It sure doesn't for the database queries on indexed rows, for <100k entries.

Instance.find() (...) doesn't contain a join.

That's why I labeled it surprising: it does, for the the preloading of instance badges.

I don't think caching the feedback instance is useful (as that would be prone to breakage once someone adds a category in there). Instead, we should only query the properties we need. I'll push out a number of small commits that each add options to test this and optimize the instance process. They will not change the behavior of adhocracy unless set.

nidico commented 11 years ago

I don't think caching the feedback instance is useful

I think you misunderstood my proposal. I'm not talking about caching between requests, but within one request, such that the query for get_feedback_instance is only constructed once and the database is only queried once (instead of 3 times, in that case) per request for this purpose.

Instance.find() (...) doesn't contain a join.

That's why I labeled it surprising: it does, for the the preloading of instance badges.

Oh, not good. Is that due to the lazy='joined' parameter in the InstanceBadge mapper?

phihag commented 11 years ago

With the pushed changes, the hhu theme doesn't do any per-request database queries for the feedback form (and if you're certain that the feedback instance is there, setting adhocracy.check_feedback_instance = False will now cause only one (albeit costly) query per request with the default (non-hhu) configuration.

nidico commented 11 years ago

What's bad about my proposal? It'd lead to better results (1 instead of 2 queries if feedback categories are used) without adding additional configuration options. Furthermore, such local caching can be used in other such places.

phihag commented 11 years ago

@nidico Well, with caching, there would still be at least one query necessary even if categories are not used, and probably (still) two (one for the instance, one for the categories) if they are. I'm not saying your proposal is bad, I just think it's it's extremely unlikely that the feedback instance is not available, but feedback is enabled, and therefore we can simply disable this check, and don't even need caching. And a correct per-request cache seems far from trivial, whereas these changes are extremely simple.

nidico commented 11 years ago

don't even need caching.

Well, we're having feedback categories enabled all around, so without caching (and without check) it's two requests plus the categories request, while with caching it's only 1+1.

And a correct per-request cache seems far from trivial

I'm thinking of a dict which is created on start of the request and emptied on the end of the request. I'm not seeing any problems with this approach. Of course you can still shoot yourself in the foot by making modifications to the database and then using data from the cache, but that's not different than using other local objects which have a database representation.

whereas these changes are extremely simple.

Agreed.

phihag commented 11 years ago

Sorry, I must have missed the second request. Apart from the obvious

(Mako) h.feedback.get_categories()
get_feedback_instance
model.Instance.find

where else do we access the feedback instance?

nidico commented 11 years ago

We don't, my fault. I thought we're using get_feedback_instance directly as well, but we don't. Fine!

phihag commented 11 years ago

The following mod_wsgi configuration should work, but needs mod_wsgi to be compiled especially for our Python:

WSGIPythonHome /home/phihag/inavm/adhocracy_buildout/python/python-2.7/

<VirtualHost *:80>
        Servername localhost

        DocumentRoot /home/phihag/inavm/adhocracy_buildout/
        <Directory />
                Options FollowSymLinks
                AllowOverride None
        </Directory>

WSGIDaemonProcess localhost user=phihag group=phihag processes=4 threads=25
WSGIProcessGroup localhost
#WSGIPythonExecutable /home/phihag/inavm/adhocracy_buildout/bin/python
        WSGIScriptAlias / /home/phihag/inavm/adhocracy_buildout/scripts/site.wsgi

    <Directory /home/phihag/inavm/adhocracy_buildout/scripts/site.wsgi>
    Order allow,deny
    Allow from all
    </Directory>

ErrorLog ${APACHE_LOG_DIR}/error.log
LogLevel warn
</VirtualHost>
phihag commented 11 years ago

On my system, mod_wsgi compilation fails with:

$ make
/usr/bin/apxs2 -c -I/home/phihag/inavm/adhocracy_buildout/python/parts/opt/include/python2.7 -DNDEBUG   mod_wsgi.c -L/home/phihag/inavm/adhocracy_buildout/python/parts/opt/lib -L/home/phihag/inavm/adhocracy_buildout/python/parts/opt/lib/python2.7/config  -lpython2.7 -lpthread -ldl  -lutil -lm
/usr/share/apr-1.0/build/libtool --silent --mode=compile --tag=disable-static x86_64-linux-gnu-gcc -prefer-pic -DLINUX=2 -D_FORTIFY_SOURCE=2 -D_GNU_SOURCE -D_REENTRANT -I/usr/include/apr-1.0 -I/usr/include/openssl -I/usr/include/xmltok -pthread     -I/usr/include/apache2  -I/usr/include/apr-1.0   -I/usr/include/apr-1.0  -I/home/phihag/inavm/adhocracy_buildout/python/parts/opt/include/python2.7 -DNDEBUG  -c -o mod_wsgi.lo mod_wsgi.c && touch mod_wsgi.slo
In file included from /home/phihag/inavm/adhocracy_buildout/python/parts/opt/include/python2.7/Python.h:8,
                 from mod_wsgi.c:142:
/home/phihag/inavm/adhocracy_buildout/python/parts/opt/include/python2.7/pyconfig.h:1173:1: warning: "_POSIX_C_SOURCE" redefined
In file included from /usr/include/sys/types.h:27,
                 from /usr/include/apr-1.0/apr.h:131,
                 from /usr/include/apache2/ap_config.h:25,
                 from /usr/include/apache2/httpd.h:43,
                 from mod_wsgi.c:34:
/usr/include/features.h:158:1: warning: this is the location of the previous definition
In file included from /home/phihag/inavm/adhocracy_buildout/python/parts/opt/include/python2.7/Python.h:8,
                 from mod_wsgi.c:142:
/home/phihag/inavm/adhocracy_buildout/python/parts/opt/include/python2.7/pyconfig.h:1195:1: warning: "_XOPEN_SOURCE" redefined
In file included from /usr/include/sys/types.h:27,
                 from /usr/include/apr-1.0/apr.h:131,
                 from /usr/include/apache2/ap_config.h:25,
                 from /usr/include/apache2/httpd.h:43,
                 from mod_wsgi.c:34:
/usr/include/features.h:160:1: warning: this is the location of the previous definition
/usr/share/apr-1.0/build/libtool --silent --mode=link --tag=disable-static x86_64-linux-gnu-gcc -o mod_wsgi.la  -rpath /usr/lib/apache2/modules -module -avoid-version    mod_wsgi.lo -L/home/phihag/inavm/adhocracy_buildout/python/parts/opt/lib -L/home/phihag/inavm/adhocracy_buildout/python/parts/opt/lib/python2.7/config -lpython2.7 -lpthread -ldl -lutil -lm
/usr/bin/ld: /home/phihag/inavm/adhocracy_buildout/python/parts/opt/lib/libpython2.7.a(abstract.o): relocation R_X86_64_32 against `.rodata.str1.8' can not be used when making a shared object; recompile with -fPIC
/home/phihag/inavm/adhocracy_buildout/python/parts/opt/lib/libpython2.7.a: could not read symbols: Bad value
collect2: ld returned 1 exit status
apxs:Error: Command failed with rc=65536
.
make: *** [mod_wsgi.la] Error 1

uwsgi does not offer significant improvements.

Instead, we should use mod_proxy_balancer.

phihag commented 11 years ago

Using a load balancer to let both nodes serve adhocracy doubles the performance. Installed in production, and modified the ./switch command to switch between node1, node2, and both.

nidico commented 11 years ago

I'm not sure about the reason for the mod_wsgi compilation - this isn't required for uwsgi. You'd either use a reverse proxy or a native uwsgi webserver module.

Note that uwsgi also contains a process manager which spawns multiple processes, and can act as a load balancer if accessed directly via HTTP.

uwsgi does not offer significant improvements.

We haven't yet played a lot with different configurations, but we'll do very soon and share the results. We're currently using mod_proxy_balancer in the more active production setups, so we can compare.

phihag commented 11 years ago

mod_wsgi was an alternative to uwsgi that we considered, and we'd use it on every node (and not the SSL terminator / load balancer in front). Without any changes to the default configuration, uwsgi does not significantly increase the sped (compared to paster) for me.