After some playing with kcachegrind I discovered that AF (and some other your modules) is very slow in loading. This made me look deeper, so I disabled AF and measured load time on some node (not related to forum!) page. This got me from ~270ms of page generation time down to ~230ms (and this is using APC opcode accelerator). And I have quite lot of modules, much more code-heavy than AF. Disclaimer: I fully understand that this is not proper test and just rough estimation. But kcachegrind results can be considered as such test.
Looking at the code indeed gives hints on reasons of such performance. It seems you don't use code separation practice and load most of the code on each request unconditionally. No wonder that your modules are slow. That means more memory is used, more CPU time is spent on loading (even while using opcode accelerator).
Examples of possible optimisation:

  • You include settings.inc non-conditionally. This code must be included by "file" feature of menu system, so menu system loads the code only when accessing admin page.
  • core-overrides.inc should be refactored: for example advanced_forum_page() should be moved to separate file and included by "file" feature too
  • style.inc should probably refactored too: if it contains only theming code no need to call it on every page. It's enough to include it conditionally when needed

Ofcourse, this is not performance audit. It's only my ideas from quick look at the code.

Comments

crea’s picture

crea commented
Title: AF Performance is bad » AF performance is bad
michelle’s picture

michelle
she/her
Primary language English
Location Wisconsin, USA
commented
Title: AF performance is bad » Split out some code to separate files to improve performance
Category: bug » task
Status: Active » Postponed

Considering this module is only in alpha, that I've already made some progress towards splitting things up and calling conditionally, and that the queries are far more of a problem than code loading, I really don't care for the tone of this issue. That said, I'm all for improving performance wherever possible so I discussed this issue with catch, who knows far more about performance than I do.

The conclusion was that it's worth splitting the page and the settings so I'll do that. Setting this to postponed until we're closer to beta. There's a lot more work that needs to get done before I worry about moving functions around.

So, thanks for the advice but next time please hold the accusatory tone. It really isn't necessary.

Michelle

crea’s picture

crea commented

I didn't mean to be accusatory..sorry if it looked like that

merlinofchaos’s picture

merlinofchaos commented

If using APC, code separation won't make a great deal of difference, I've found, so I'm not sure that's really where the problem lies. I also took a glance at some of AF's code, and load-on-demand might be kind of difficult for some of it, particularly the theming parts, where it will be loaded from the registry and the theme registry is great at loading the normally registered stuff, but AF is using an alter hook to add its stuff in and that can't be loaded on demand as easily.

So some of the page code could be made more on demand, but that only really affects the general 'forums' page. I'm not sure anything used in node display can be moved. Perhaps a little, but the effort to reward ratio might not be good unless some solid data shows a real problem in a particular spot.

michelle’s picture

michelle
she/her
Primary language English
Location Wisconsin, USA
commented

@crea: Maybe it's just a language thing but when you title the issue "AF performance is bad" and say stuff like "No wonder that your modules are slow." when you're talking about 40 MILLIseconds, it starts things off on the wrong foot, no matter how good your intentions.

@merlinofchaos: Thanks for the input. Definitely helps hearing from folks who know more about this than I do.

@all: One thing to consider is that AF's styles can be used on more than just the forums. On my site, for example, I use AF to style the forums, groups, blogs, and images. So I need to have AF style code be available on nearly every page load all over the site. Others may not use it to that extreme but I wouldn't be surprised if the option to style all comments like forum ones wasn't fairly popular as it gives a nice, uniformed, look.

Michelle

crea’s picture

crea commented

@Michelle I apologize for calling your modules "slow". This needs some hard numbers to base upon, indeed. 40 ms difference is bad if you use relative scale and compare to total time, but unfortunatly I cannot reproduce it with AB (apache bench). It reports different numbers than devel. That's why I said it's not some kind of test but the thing that hooked me on looking at the code. Making good test involves lot of time and preparations.
Even if we ignore page loading time, optimisation can help with memory usage and it's good that we have agreement here about separating code. Until PHP gets garbage collector we need to reduce loading of unused code to the minimum :)

@merlinofchaos: yes deep refactoring may be unneeded but moving functions "as is" between includes is not hard and is quite rewarding. Copy-paste ftw.
I wonder if it's possible to put include_once() straight inside big theming functions, cutting all code and putting it in include file ? Wouldn't that work even with registry alter ?

michelle’s picture

michelle
she/her
Primary language English
Location Wisconsin, USA
commented

I wonder if it's possible to put include_once() straight inside big theming functions, cutting all code and putting it in include file ? Wouldn't that work even with registry alter ?

Yes, and I've already done that. I spent quite a bit of time looking at what could be put into separate files a while back but decided to put further optimizing on hold until I had more functionality in place.

Michelle

crea’s picture

crea commented

Btw, Michelle, any chance of further splitting AF styling part ? You mentioned that you need styles loaded, and that makes sense for many people including myself. I decided to use forum styling for my OG discussions, and I plan to get rid of forum eventually (with 100+ modules already on my site every module I can get rid of counts). What is stopping me now is AF depends on forum and is monolithic. It would be nice to be able to apply AF styles without enabling most forum-related code.
If AF remains as it is now I will have to split out style parts myself and abandon AF, and I would prefer not to do that..

michelle’s picture

michelle
she/her
Primary language English
Location Wisconsin, USA
commented

80% of AF is code to feed variables to the styles... There's really not much sense in splitting it up into a separate module. If you end up splitting something up, I'd be willing to take a look and see if it makes sense for the module but it's not something I want to spend any time on.

Michelle

michelle’s picture

michelle
she/her
Primary language English
Location Wisconsin, USA
commented
Status: Postponed » Closed (won't fix)

I split out quite a bit over the years and will likely do more refactoring as I go along but no real need to keep a specific issue around for this.

Michelle