The model is responsible for getting the articles. Not the module itself. If you create queries everywhere instead of using the models the code will be a mess.

If you want to speed up things. Use caching. Or you should improve the model itself, if possible.

I'm not a fan of complex queries in modules.
I wonder why it takes that much longer to run basically the same query. Maybe the model could be improved instead?

This is my first contribute to Joomla, so I chose the easy way. But you are right, I'll check if the model can be improved without blowing up things.

The easy way can sometimes be a good way to find a desired end state, in which case you found one with a more performant database query. I'm assuming you based the query in this pull on the code in the model, so try using some of what you modified in the model and see if you can improve the code there. The end result is if you can improve the query there, you improve the query for several areas of Joomla and not just a single module's instance.

No, I wrote it from scratch.
The module shows the most read articles, it doesn't have to fetch user info or ratings, so I can remove the two joins on the user table (on "created_by" and "modified_by", and the second field isn't indexed).
The join on the parent category (of the article's category) is unneeded, the module builds the route with "category_alias", but not "parent_alias".
Then, there is a join on "#__content_frontpage" that seems totally useless, in the model it doesn't appear in SELECT or WHERE clause and I can't find any occurrence.
In the end, even the "badcats" check should be useless, as all the subcategories get unpublished when you unpublish the parent category in the administration panel.

I can try to improve the model, but it will never be as fast as a dedicated query. The model needs those joins.

Ok, I changed the model to prefetch bad categories before the real query. The model now runs two queries, but this has a HUGE impact. It went down to 1.69+19.36 ms.

Compared the performance of this on my own site's blog page with debug mode enabled for my logged in account.

Pre-patch: One query ran in 31.42ms with 0.022 MB memory use
Post-patch: Two queries ran in 0.08ms & 1.70ms with 0.023 MB & 0.022 MB memory use

Looks like a good patch to me.

This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/4689.

@test It's awesome with lots of articles :)

This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/4689.

It may also be an option to add an additional "mode.simple" (or whatever) state to the model which can be set in the module. Basically making some joins optional if the data isn't needed.

@test
Site staging, 3.3.7, freshly installed with test data
After applying this patch, I get this warning on pages "Most read" and "Latest Articles":
Warning: Invalid argument supplied for foreach() in D:\www\joomla-staging\joomla-cms\components\com_content\models\articles.php on line 565

This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/4689.

And two more:
Warning: Invalid argument supplied for foreach() in D:\www\joomla-staging\joomla-cms\modules\mod_articles_popular\helper.php on line 66
Warning: Invalid argument supplied for foreach() in D:\www\joomla-staging\joomla-cms\modules\mod_articles_popular\tmpl\default.php on line 13

This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/4689.

Your database is MySql or SQL Server?
I think there is a problem in the query, so you get the "same" error every time it tries to load the missing object array.

MySQL - never used anything else ...

Ok, I got it... If there isn't any bad category, the IN() clause is empty and the query fails. But this suggests even more improvements. I'll work on it.

Based on the comments above I am resetting the tests so that the new code gets tested - thanks

This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/4689.

Ok, I messed something with Netbeans, so consider just "d764790" and "881ccc7".
Anyway, I added the "ignore.users" and "ignore.ratings" states to remove joins on users and ratings tables when not needed as suggested by Bakual. Now it should be safe to test.

I know this is old so can I be a bad human and ask that you merge back in staging so I can get people to test this please :) Would love to see this make it into Joomla 3.4!

Ok, I'll close this pull request and open another one (I'm not good in git's black magic)