Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding template cache handling #146

Open
wants to merge 2 commits into
base: gh-pages
Choose a base branch
from

Conversation

Broatcast
Copy link

Adding some examples to the template handling for the template cache system.

Adding some examples to the template handling for the template cache system.
Copy link
Contributor

@JoshHarmon JoshHarmon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hopefully this doesn't come off too much as me being a grammar pedant, but I feel there are a few changes that should be made before this page gets pulled into the docs.

Otherwise, great work! Thanks for submitting it!

@@ -94,3 +94,46 @@ global $user;

$user['favorite_colour'] = 'Blue';
```

## Caching Templates
Don't forget to cache the template. This is important for pages with big load and prevent the database server from extra querys.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

querys --> queries

Don't forget to cache the template. This is important for pages with big load and prevent the database server from extra querys.

### Caching a template
An simple way to cache an template only at the place where it's needed, is to use the `THIS_SCRIPT` definition.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"An simple way" --> "A simple way"

The general rule for a vs. an is that if the word after it starts with a vowel, an is used. Otherwise, a is typically used.

Also, the comma here isn't necessary. "is to use..." is still part of the same clause.

}
```

### Testing if the template get cached
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"gets cached" or "got cached" or "templates get cached"


### Testing if the template get cached
You can simply test if the templates get cached correctly or not. Open the page where they should get cached and add a `?debug&debug=1` or `&debug&debug=1` to the url into the browser.
Now you see the `MyBB Debug Information` page. The first table called `Page Generation Statistics` contain a row with the template cache information.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"the first table ... contain" --> "the first table ... contains"

You can simply test if the templates get cached correctly or not. Open the page where they should get cached and add a `?debug&debug=1` or `&debug&debug=1` to the url into the browser.
Now you see the `MyBB Debug Information` page. The first table called `Page Generation Statistics` contain a row with the template cache information.

#### There is an template that not get cached:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider changing to "If a template does not get cached" or something and remove the colon. Headings don't need colons at the end of them.


#### There is an template that not get cached:
`No. Templates Used: 90 (89 Cached / 1 Manually Loaded)`
This means there is 1 template that need an extra select query to the database to get loaded. At the botton you see the `Template Statistics` and now can control wich templates get not cached. All template names in `Templates Requiring Additional Calls (Not Cached at Startup) - 1 Total` are not cached, so you should correct your code.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"template that need an extra select query" --> "template that needs an extra select query"

Typo: "wich" --> "which"

"get not cached" --> "don't get cached" or "are not cached" or similar.

`No. Templates Used: 90 (89 Cached / 1 Manually Loaded)`
This means there is 1 template that need an extra select query to the database to get loaded. At the botton you see the `Template Statistics` and now can control wich templates get not cached. All template names in `Templates Requiring Additional Calls (Not Cached at Startup) - 1 Total` are not cached, so you should correct your code.

#### If you do all right and the templates get cached at tho correct place it looks similar to:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps change to something like: "How to determine if templates are being cached correctly"

Headings shouldn't be the dependent clause of what would otherwise be sentences.

@Broatcast
Copy link
Author

Big thx @JoshHarmon ... like i sayed english is not my main language and i'am feel my english stills realy bad... so yes please some one correct my gramma and other failures ;)

Correcting typos and gramma, thx to @JoshHarmon
@JoshHarmon
Copy link
Contributor

Looks good to me! Just have to wait and see if any team members stop by and have any feedback before pulling it in.

Copy link

@Shade- Shade- left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are some corrections I would like you to apply before merging. Not to be offensive, but the language you used did not look very professional and some information was missing, so I have rewritten most of the commit on top of your base.

@@ -94,3 +94,46 @@ global $user;

$user['favorite_colour'] = 'Blue';
```

## Caching Templates
Don't forget to cache the template. This is important for pages with big load and prevent the database server from extra queries.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caching templates is important to optimize the load on your server's database and to avoid performing extra queries.

Don't forget to cache the template. This is important for pages with big load and prevent the database server from extra queries.

### Caching a template
A simple way to cache an template only at the place where it's needed is to use the `THIS_SCRIPT` definition.
Copy link

@Shade- Shade- Mar 23, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In order to cache a template you just need to add it to the $templatelist variable, a coma-separated list of templates which is then used by MyBB's core to fetch templates using a single database query. The aforementioned fetch occurs in global.php and the only hook available before is global_start. A proper way to cache a template would then be:

$plugins->run_hooks('global_start', 'myplugin_cache_templates');
function myplugin_cache_templates()
{
    global $templatelist;

    if(isset($templatelist))
    {
        $templatelist .= ',';
    }
    else
    {
        $templatelist = '';
    }

    $templatelist .= 'hello_world_template';
}

You typically use templates in specific locations. You can avoid caching templates globally and cache them in certain pages by checking against the THIS_SCRIPT definition. For example, a template might be needed just in the postbit, therefore you can check if the user is watching showthread.php or not:

if(THIS_SCRIPT == 'showthread.php')
{
    // Cache templates
}

```

### Testing if the template gets cached
You can simply test if the templates get cached correctly or not. Open the page where they should get cached and add a `?debug&debug=1` or `&debug&debug=1` to the url into the browser.
Copy link

@Shade- Shade- Mar 23, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can test whether if templates get cached correctly or not with ease by opening the page where they should be cached and appending the ?debug=1 query string to the URL. Use &debug=1 if there are already other query strings in place. The resulting MyBB Debug Information page contains a table called Page Generation Statistics which has a row with the template cache information.

Note that only administrators can access the debug page, so ensure you are logged in with an administrator account.


#### If a template does not get cached
`No. Templates Used: 90 (89 Cached / 1 Manually Loaded)`
This means there is 1 template that needs an extra select query to the database to get loaded. At the botton you see the `Template Statistics` and now can control which templates don't get cached. All template names in `Templates Requiring Additional Calls (Not Cached at Startup) - 1 Total` are not cached, so you should correct your code.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A template not cached is added to the Manually Loaded count. An extra select query to the database is performed for every template in such list. The complete list of templates requiring additional calls is located within the Template Statistics section.

`No. Templates Used: 90 (89 Cached / 1 Manually Loaded)`
This means there is 1 template that needs an extra select query to the database to get loaded. At the botton you see the `Template Statistics` and now can control which templates don't get cached. All template names in `Templates Requiring Additional Calls (Not Cached at Startup) - 1 Total` are not cached, so you should correct your code.

#### How to determine if templates are being cached correctly
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not necessary IMHO.

@Ben-MyBB
Copy link
Member

@Broatcast Could you update your PR?

@Broatcast
Copy link
Author

I will update it in a few days ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants