- Add new method privatemsg_message_load_multiple() which can load multiple messages in one query and then loops over the load hook.
- Renamed _privatemsg_load to privatemsg_load_message() to be consistent. this function is now simply a wrapper for multiple (this is what core is doing with user/node_load_multiple
- Added static cache for thread_load() - As I noticed it is called twice. I could also rip that into an own patch.
This kills a lot of queries, especially because of the static cache.. a lot user_load() and follow queries.
| Comment | File | Size | Author |
|---|---|---|---|
| #11 | privatemsg.load_multiple6.patch | 6.3 KB | berdir |
| #9 | privatemsg.load_multiple5.patch | 4.42 KB | berdir |
| #7 | privatemsg.load_multiple4.patch | 4.89 KB | berdir |
| #6 | privatemsg.load_multiple3.patch | 5.01 KB | berdir |
| #2 | privatemsg.load_multiple2.patch | 4.93 KB | berdir |
Comments
Comment #1
litwol commentedAs per description of the patch, it seem to add more than one feature. I advise to split the patch per feature. It will become easier to review as well as easier to track issues if any become introduced.
Thanks.
Comment #2
berdirRe-roll, third item of that list splittet into an own patch at #444672: static cache for thread_load
Comment #3
naheemsays commentedTested and this also works as expected, code also looks sane and well documented, but it is a more involved patch, so will leave at cnr for now.
Comment #4
naheemsays commentedI have been using this and 7 privatemsg other patches on my site for quite a few days now with no problems. Settings to rtbc.
Comment #5
naheemsays commentedThis patch breaks messages/new/% with the following error:
Comment #6
berdirAdded a check to see if an empty array is passed in to load_multiple.
Comment #7
berdirTested for the wrong variable, should be fixed now.
Comment #8
naheemsays commentedYes this works.
The problem was that the menu system was calling privatemsg_thread_load which was calling privatemsg_message_load_multiple without any pmid's and that was throwing up an error. That has been fixed.
I do think there is a deeper problem, probably with the core menu system though as messages/views/%privatemsg_thread should IMO NOT be acting on all Messages/%/% paths.
Comment #9
berdirOk, here is a re-roll of that patch.
Funny, I used ...2.patch but then re-discovered and re-fixed the empty $pmids bug again in pretty much the same way, just with a better comment I think, before looking at this issue :)
The re-roll was not easy as there were quite some changes, so this needs to be tested again, but it seems to work as expected for me. setting back to needs review.
Comment #10
naheemsays commentedThis still works well and makes sense.
Comment #11
berdirThanks to the new tests, I was able to catch and fix a few bugs of the patch. Attached is a re-roll.
I want this for 1.0, but it's quite a deep change and I'd like to have some more tests/reviews from others.
Comment #12
berdirAdded to 7.x-1.x-dev and 7.x-1.x-dev in the hope that the tests are good enough :) This should be a great performance inprovement for big threads.