The 2026 Drupal Business Survey is open. Your response will help shape the future of the Drupal business ecosystem.While reviewing the performance of a client website, I noticed that the same identical queries from userpoints_get_max_points() are run multiple times on same pages, often >25 times. The attached patch fixes this.
On the page where I tested this, it resulted in ~100 fewer database queries to build the page.
| Comment | File | Size | Author |
|---|---|---|---|
| #3 | userpoints.module.patch | 1.03 KB | jeremy |
| userpoints.module.patch | 1.04 KB | jeremy |
Comments
Comment #1
jeremy commentedFixing title.
Comment #2
kbahey commentedI like the idea, but I think your logic has a problem.
The function can be called for a different $uid each time. So caching has to take that into account.
Right now it will return the value for the first $uid that is cached regardless of which $uid is passed in.
Also, I think the code could be simpler. For example return early if found in the cache.
As well, roll it against what is in DRUPAL-6--1 cvs tag.
Comment #3
jeremy commented> The function can be called for a different $uid each time. So caching
> has to take that into account.
Whoops! Indeed, an updated patch is attached.
> Also, I think the code could be simpler. For example return early if
> found in the cache.
I don't see any way to get a gain from this. The logic does the following:
1. if uid is not set, set it
2. if tid is not set, set it
3. if cached value is not set, set it
4. return cached value
You could add a 2.5 that returns if the cache is set, but that means 1) you'd add an if to save an if which saves nothing, and 2) you'd have to return twice in this function.
> As well, roll it against what is in DRUPAL-6--1 cvs tag.
Okay, done.
Comment #4
kbahey commentedEarly return seems to be the norm in Drupaland, although like you, I used to shy away from it.
Anyway, it is now commited, with lots of comments for clarity.