SimplePortal
Development => Bugs => Fixed or Bogus Bugs => Topic started by: emanuele on September 18, 2012, 05:38:45 PM
-
Hello all! :D
This is my first post here and I really feel I used the wrong board, but...well...it seems the less inappropriate. O:)
Now the question/customization I have.
While looking at the details about queries and page creation to minimize the number of queries (yes, I'm using a free host that limits the hourly number of queries, so I was looking around if I was able to spot some place to reduce the count, not that I need it, but better have some space), I noticed that when creating a page SP there were 35 queries... *headscratch* ...strange, at first I thought it was AEVA and the thumbnails on the page, but enabling the debug I noticed that SP was repeating the very same query something like 16 times in a row. The query is/was the one in sportal_get_pages.
Since it's basically the first time I'm using SP (and a portal in general TBH), I'm not exactly sure of the reason, so I changed the code in a hopefully conservative way that should break anything to:
function sportal_get_pages($page_id = null, $active = false, $allowed = false)
{
global $smcFunc;
static $returns = array();
$query = array();
$parameters = array();
if (isset($returns[$page_id]))
$return = $returns[$page_id];
else
{
if (!empty($page_id) && is_numeric($page_id))
{
$query[] = 'id_page = {int:page_id}';
$parameters['page_id'] = (int) $page_id;
}
elseif (!empty($page_id))
{
$query[] = 'namespace = {string:namespace}';
$parameters['namespace'] = $page_id;
}
if (!empty($active))
{
$query[] = 'status = {int:status}';
$parameters['status'] = 1;
}
$request = $smcFunc['db_query']('','
SELECT
id_page, namespace, title, body, type, permission_set,
groups_allowed, groups_denied, views, style, status
FROM {db_prefix}sp_pages' . (!empty($query) ? '
WHERE ' . implode(' AND ', $query) : '') . '
ORDER BY title',
$parameters
);
$return = array();
while ($row = $smcFunc['db_fetch_assoc']($request))
{
if (!empty($allowed) && !sp_allowed_to('page', $row['id_page'], $row['permission_set'], $row['groups_allowed'], $row['groups_denied']))
continue;
$return[$row['id_page']] = array(
'id' => $row['id_page'],
'page_id' => $row['namespace'],
'title' => $row['title'],
'body' => $row['body'],
'type' => $row['type'],
'permission_set' => $row['permission_set'],
'groups_allowed' => $row['groups_allowed'] !== '' ? explode(',', $row['groups_allowed']) : array(),
'groups_denied' => $row['groups_denied'] !== '' ? explode(',', $row['groups_denied']) : array(),
'views' => $row['views'],
'style' => $row['style'],
'status' => $row['status'],
);
}
$smcFunc['db_free_result']($request);
$returns[$page_id] = $return;
}
return !empty($page_id) ? current($return) : $return;
}
It seems to work as expected and the number of queries is dropped from 35 to 16, but still I'd like to ask you: does it make sense?
Did I miss anything obvious?
Thank you for any answer! ;D
Bye!
Emanuele
-
Welcome to Simple Portal, Emanuele.
That is an excellent question. The static array functions as cache.
We all know that SMF has benefitted from caching various semi-static information, and your example is a proof that this concept can also work for Simple Portal.
A complete implementation of a cache would prevent errors that would naturally arise when the page namespace string changes, or when the page is deleted from the database.
A complete understanding of the effect of performance of a fully-implemented cache on page management would follow from a benchmark study with and without page information caching.
Both SiNaN and Nathaniel have put lots of work into building various improvements into Simple Portal in the past; I am guessing that page information caching was one of those things that was not implemented because it was not seen as high-need -- the application is not optimized for use on a host such as yours, which puts limits specifically on the number of db queries. Of course, many of the Simple Portal blocks, by their very nature, add tremendously to the number of DB queries required to create pages.
I cannot speak for SiNaN's plans or priorities for future development of Simple Portal. In the past, I have seen him show interest in well-thought-out and well-implemented innovations for Simple Portal.
-
Well, it's not that I'm *so* limited at the moment: 15000 queries/hour and an average of 200/300 page views a day, so far from any limit. Even if I'd have 300 page views in one hour and 30 queries per page I would still be below the limit, so it's fine on that respect.
That one caught my attention because it was...weird. :P
-
I think that most of the calls to sportal_get_pages are to check each block on the blocks list to find out if it should be shown on that page.
Here is how I checked this:
I created a new BBC page, saved it, and viewed it. 25 queries. No blocks are displayed on this page.
I changed the title and viewed it again. 26 queries.
I added a line of text to the body and viewed it again. 24 queries.
So I tried $db_show_debug to show what was going on.
In this case, I observed that SQL statement found in function sp_get_pages was executed 19 times.
1 the sp_get_pages SQL statement was executed
2 Second, the SQL statement from function getBlockInfo was executed
3 Then, the sp_get_pages SQL statement was executed 18 more times.
Just to follow up on this, I added some debugging info to $context, just before each call to sp_get_pages , and some debugging stuff to echo that out in the page template.
I observed that, first, getShowInfo called sportal_get_pages 18 times, then sportal_pages called sportal_get_pages one time.
Now there are 16 blocks on that forum, so that accounts for 16 of the calls (each block has to be checked to see if it belongs on the page). That does not account for the other 2 calls from getShowInfo to sportal_get_pages.
The use of static for caching this information could offer an advantage in the situation where there are a large number of active blocks which must be checked.
What is the downside of using a static variable, as opposed to SMF caching?
Presumably, deleting a page in the middle of all this checking, or changing its "namespace", could cause the statically-stored information to suddenly become incorrect. Of course, this situation should also cause other problems in rendering the page.
But these situations are anticipated to be rare. It would take some careful consideration to come up with potential security issues (which would probably have to involve 3 "namespace" changes in quick succession), and decide whether use of a static variable in this way would represent a threat.
The upside of using the static variable is that it is very easy to implement, and does not involve quite as much overhead as using SMF caching.
-
You can use the cache when you expect that you have 1) a "slow" query (or at least demanding) and 2) you use this query frequently in different page loads.
You can use a static variable when you know you are accessing the exact same information over a single page load.
In other words, the advantage of caching is evident if you have a page that is frequently accessed and is performing a rather heavy query, in that case, the time taken by the query is mostly equivalent to check the file to include exists, include it and use the resulting variable (because anyway even with the SMF file-caching (other kind of caching of course are not affected by this issue) there is the huge overhead of a disk access (instead of accessing the database you are accessing the filesystem to include a file...and in certain cases that could be even slower than a query).
The advantage of a static variable is evident in cases like this when you have a pretty innocent function called to access the same information over and over during a single page load (because of course when the page load is completed the variable is gone and it cannot be re-used next time). In that case the query is pretty "innocent" (i.e. shouldn't create any relevant load on the server), though is executed several times during the same page load, using file cache would create probably more issues than it solved because it would require 16 file_exists (that are slow on their own) plus some other things.
An easy analogy is with a file: keep a file always open (quickly accessible, but if you switch off the computer is gone) or save it to the disk (slower access, but if you switch off the computer you can find it again).
-
I hear you.
The danger of change between calls to the static variable is quite small.
So the savings of resource is probably worth it, if you can be sure the risk of "name rotation" won't lead to private stuff being displayed to people who should not see it.
-
Fixed for the upcoming version.
This was just a silly oversight at my end. Thank you for the report and suggestion.
-
:D
Glad it was helpful!