SQL & php Spam performance changes for 20240308.1

[joeiacoponi1]

[I do not appear to have write access to My-Little-Forum, so I created a fork and a pull request off it.]

4 total includes files modified: main, entry, index, thread. All changes relate to queries using display_spamquery(and or where).

Two basic assumptions made: 1) All query results in base 20240308.1 are correct.
2) The great majority (if not all) of MLF forums will have a far greater number of spam = 0 entries in akismet and b8 than spam = 1 entries.

Changes For the code changes, given assumption 2 above (far fewer spam = 1 entries), I placed the retrieval of akismet and b8 records within a SQL subquery and only retrieved those where spam = 1. I then joined those results with the base query and changed the JOIN parameters depending on whether show_spam is set and whether we are returning individual entries or entire threads.

While performing the SQL changes, I noticed that the same basic SQL queries are repeated in a few instances, only differing based on if statement results, with the individual SQL statements run within the if statement. The modifications I made consolidate the code a fair bit by specifying the base SQL code to be used for all scenarios, and then only setting an additional variable within the if statement results. I then moved the SQL statement to run outside / after the if statement.

Testing I have completed testing across all variables affected within these queries: a) User: Logged in vs out b) Categories: No Categories specified - display all, Categories specified - display all, Categories specified - display one c) Spam: No Spam entries, Spam entries exist and (not logged in or session(show_spam) not set), Spam entries exist and session(show_spam) is true.

Tested pages / modes / actions were index, user, show_posts, thread, posting and admin.

All observed results (threads, posts, latest postings, total postings, total threads) for these changes match the results in base 20240308.1.

In all instances tested, page load time went from 10-15 seconds in base 20240308.1 down to sub 1 second with these modifications. I do see about a 40% increase in page load time (from 500ms to 800ms avg) over 2.4.24, but page load is still acceptably under 1 second, even within our forum of 500,000 posts / 40,000 threads.

Missing Index Note Responding to Auge's question about the missing akismet indexes during the double upgrade, I did add these indexes to the DB manually early during my testing process. The indexes did not materially affect performance of base 20240308.1, but did dramatically affect performance with these pull request modifications in place.

Joe

[loesler]

Thank you for effort! Just a question on handling the changes correctly: Do we have to reconstruct #705?

[joeiacoponi1]

Thank you for effort! Just a question on handling the changes correctly: Do we have to reconstruct #705?

You're quite welcome, Micha. It was a good exercise, and hopefully others can find value in it.

Good catch. My modifications were based off master from 3/22, and I see that #705 was committed on 3/25. Given that, yes we will need to merge the changes from this and #705 within includes/main.inc.php. There doesn't appear to be any overlap in the changes between the two pull requests, so the merge should be straightforward.

Joe

[joeiacoponi1]

I've now committed the #705 changes to includes/main.inc.php to my master branch, so the pull request should be correct.

Joe

[loesler]

What a service. Thank you. ;-)

[auge8472]

I'll review the changes today or tomorrow. @loesler Feel free to check the changes yourself too. Four eyes can see more than two.

[joeiacoponi1]

Thank you for catching those Micha. I had done a code compare / merge, but for some reason those two didn't get moved over.

[auge8472]

I've now committed the #705 changes to includes/main.inc.php to my master branch, so the pull request should be correct.

This PR does not contain the changes of #705 (with stand of now (2024-04-19 14:24 CEST)). If you pulled the changes of #705 and merged it into your own master-branch and it is in a current state like our master-branch (plus your changes to this PR), you have to transmit those changes to your master on Github. If git reports a conflict, you have to push it with git push -f origin master (-f to force an overwrite of the remote repo).

[joeiacoponi1]

Why did you add the main.inc.php outside the directory includes? This must be removed.

This PR does not contain the changes of #705 (with stand of now (2024-04-19 14:24 CEST)). If you pulled the changes of #705 and merged it into your own master-branch and it is in a current state like our master-branch (plus your changes to this PR), you have to transmit those changes to your master on Github. If git reports a conflict, you have to push it with git push -f origin master (-f to force an overwrite of the remote repo).

Sorry about that, a little rusty with GitHub. I've now moved main.inc.php from root back into /includes, and this does have the #705 changes along with all my changes from what the latest commit "Move main.inc.php back to includes" shows. I also removed the .DS_store files that my Mac had added.

Hopefully that cleans it all up and thanks for your assistance guys.

Joe

[auge8472]

As far as I can see, there is nothing left. IMHO @joeiacoponi1 solved all requests. Nevertheless, Github believes that another change is still pending. @loesler, @joeiacoponi1 Please take a look and tell me please what I am missing.

[edit] Obviously, I not only have to mark every single comment as solved, but I also have to approve it at the bottom, in the last block on this page. [/edit]

If there's nothing, this is ready to be squashed. 😀👍🎉

[auge8472]

... aaaaand done. 🎉

[joeiacoponi1]

That last Github pending change may have been /includes/functions.inc.php that I pulled into my repo from master, which includes the cookie_options function. I needed that for my test site to work. Regardless, this is great.

[loesler]

Good work!