Pete Jones Posted June 14, 2016 Share Posted June 14, 2016 We have a big selector which we have broken down into 3 chunks to return a list of notes (pages) with repeaters as follows. We also allow the user to filter the results. The problem we have is that the page currently takes nearly 10 seconds to process results. Is there anything we can do to improve the performance of this? I wonder if it would be worth bringing the filters into each of the find()s. I assume that caching here wouldn't work due to querystring parameters? $selector = "template=horse-note"; // Notes with unread comments (date order, most recent first) $notes_with_unread_comments = $pages->find("{$selector}, h_notes_comments.count>0, h_notes_comments.{$session->unread_by}>0, sort=h_notes_last_comment"); //echo 'Notes with unread comments ('.count($notes_with_unread_comments).'):<br />'.$notes_with_unread_comments.'<br /><br />'; // Unread notes (date order, most recent first) $notes_unread = $pages->find("{$selector}, {$session->unread_by}>0, sort=h_notes_last_comment"); //echo 'Notes unread ('.count($notes_unread).'):<br />'.$notes_unread.'<br /><br />'; // Read notes in date order (most recent first) that they were either added or that the last comment was made, whichever is most recent. $notes_other = $pages->find("{$selector}, sort=-h_notes_last_comment"); //echo 'Notes other ('.count($notes_other).'):<br />'.$notes_other.'<br /><br />'; // create notes PageArray $notes_total = new PageArray(); $notes_total->add($notes_other); $notes_total->prepend($notes_unread); $notes_total->prepend($notes_with_unread_comments); // FILTER // sanitize inputs $horse = $sanitizer->text($input->get->horse); $category = $sanitizer->int($input->get->category); $from_date = $sanitizer->text($input->get->from_date); $to_date = $sanitizer->text($input->get->to_date); $comments = $sanitizer->int($input->get->comments); // horse name if($horse) { $selector .= ", parent.h_name%=$horse"; } // note category if($category) { $selector .= ", h_notes_category_id=$category"; } // from date if($from_date) { $selector .= ", h_notes_last_comment>=".strtotime("$from_date 00:00:00"); } // to date if($to_date) { $selector .= ", h_notes_last_comment<=".strtotime("$to_date 23:59:59"); } // comments if($comments) { $selector .= ", h_notes_comments.count>0"; } // apply filter if($selector!='template=horse-note') { $notes_total = $notes_total->find($selector); } // slice PageArray according to pageNum $pageNum = $input->pageNum; $limit = 15; $start = ($pageNum-1)*$limit; $notes = $notes_total->slice($start, $limit); Link to comment Share on other sites More sharing options...
Jan Romero Posted June 14, 2016 Share Posted June 14, 2016 Why don’t you do the three database finds after building the selector? I would imagine that to speed things up, because you won’t be loading as many pages just to throw them out later. Instead of slicing the PageArray you can also put limit and start into the selector like so "start={$start}, limit={$limit}". Edit: Actually, maybe I’m missing something, but it looks like you’re getting ALL horse-notes in $notes_other. I’m also pretty sure that PageArrays automatically filter out duplicates, so when you prepend the other two PageArrays, you’re not really doing anything. 2 Link to comment Share on other sites More sharing options...
LostKobrakai Posted June 14, 2016 Share Posted June 14, 2016 You way does load all of those notes into memory even though only 15 are used later. Build your Selector first and then query the db for the total result. 3 Link to comment Share on other sites More sharing options...
arjen Posted June 14, 2016 Share Posted June 14, 2016 Also you might want to look into building a selector with arrays, since that is more readable. And if you only want to count the pages, you could also use the count method. That is blazing fast. We're calling more than 80 count method on one pageload searching around 40k pages and it is loads within 1 second. 4 Link to comment Share on other sites More sharing options...
Pete Jones Posted June 14, 2016 Author Share Posted June 14, 2016 I'm not sure I can do what we need with a single selector. The reason we ended up creating a pageArray was because we couldn't get the pages to be in the order we needed with a single selector. The refinement selectors are only used if the user chooses one or more of the filter options, so 99% of the time these won't be applied. We're grabbing all pages and then using 'slice' to grab the portion of pages for the current page. E.g., if the page number is 10, we want slice(9*15, 15) of the total pages. I know count is a lot quicker, but can't see how we can use that usefully here. Happy to take any advice though. Link to comment Share on other sites More sharing options...
adrian Posted June 14, 2016 Share Posted June 14, 2016 Honestly haven't checked to see if it is appropriate to your needs, but have you tried findMany(): https://processwire.com/blog/posts/find-and-iterate-many-pages-at-once/ Link to comment Share on other sites More sharing options...
LostKobrakai Posted June 14, 2016 Share Posted June 14, 2016 If the order is your issue than use count() with those selectors, the current page num and some math to determine which pages are to be shown on the current page - then load just those. Edit: I've not tested it with real pages, but the test class seems to work correctly: https://github.com/LostKobrakai/Paginator 10 Link to comment Share on other sites More sharing options...
renobird Posted June 14, 2016 Share Posted June 14, 2016 Dude! Nice one LostKobrakai... 1 Link to comment Share on other sites More sharing options...
Pete Jones Posted June 15, 2016 Author Share Posted June 15, 2016 Indeed, many thanks for putting that together for us LostKobrakai. Looking at my original code, can I now do this? $result = $paginator(array( "template=horse-note, h_notes_comments.count>0, h_notes_comments.{$session->unread_by}>0, sort=h_notes_last_comment", "template=horse-note, {$session->unread_by}>0, sort=h_notes_last_comment", "template=horse-note, sort=-h_notes_last_comment" ), $input->pageNum, 15); Link to comment Share on other sites More sharing options...
arjen Posted June 15, 2016 Share Posted June 15, 2016 Very nice LostKobrakai! Link to comment Share on other sites More sharing options...
LostKobrakai Posted June 15, 2016 Share Posted June 15, 2016 @Pete At least in theory it should work that way. As stated above I hadn't had time to test if further than the plain array testclass. Would be nice if you could report back if it works. If so I'd be also interested how much render time it saved you. Link to comment Share on other sites More sharing options...
Pete Jones Posted June 15, 2016 Author Share Posted June 15, 2016 Just giving an update on this. We are getting results. Page load time is now 1 second cf. 9 seconds using the original code. The results are not in the same order though. We did have problems getting them into the correct order which is why we had do this: // create notes PageArray $notes_total = new PageArray(); $notes_total->add($notes_other); $notes_total->prepend($notes_unread); $notes_total->prepend($notes_with_unread_comments); Can we influence the order in $paginator? Link to comment Share on other sites More sharing options...
LostKobrakai Posted June 15, 2016 Share Posted June 15, 2016 It's taking each selector separately one after another, so it shouldn't bring anything out of order – it shouldn't even query later selectors if the ones before where enough to fulfill the limit of pages for the current pageNum. Just make sure it's in the correct order in the first place. Link to comment Share on other sites More sharing options...
Martijn Geerts Posted June 15, 2016 Share Posted June 15, 2016 I don't know what the context of this search query is, but if it is from a search box, you could consider to use a Google Custom Search Engine. When the search queries become 'to complex' I would go for that option. Google gives back the URL, together with other found data. You could use the URL to get back to your own 'ProcessWire' data and enrich the result as wished. When you are scared having to pay for the searches, you could always store the google results temporary with MarkupCache or WireCache. 1 Link to comment Share on other sites More sharing options...
Pete Jones Posted June 15, 2016 Author Share Posted June 15, 2016 The search query returns has some specific ordering which we've been unable to create using a single selector. When the page first loads there are no filters applied so we are querying 6000 items (and counting). We need the pagination to work without having to query everything. Seems strange that the current results are not coming back in the same order as the previous query. Could it be that we are prepending the results rather than appending? What is the default order of the array? Link to comment Share on other sites More sharing options...
LostKobrakai Posted June 15, 2016 Share Posted June 15, 2016 My pagination class has no concept of appending or prepending. It's just selector after selector after selector. Just put your selectors in the intended order from the start and you should be fine. If the first selector does not return any items it'll just skip to the next one. 2 Link to comment Share on other sites More sharing options...
Pete Jones Posted June 16, 2016 Author Share Posted June 16, 2016 Ah ok, we had a few issues with the ordering, prepending vs appending and also the order of the items in each selector. Will have a tinker with it. Link to comment Share on other sites More sharing options...
Pete Jones Posted June 17, 2016 Author Share Posted June 17, 2016 The ordering is fine with the first 2 selectors in the paginator. The third one breaks the ordering for some reason.So this is fine: $notes = $paginator(array( "template=horse-note, h_notes_comments.count>0, h_notes_comments.$session->unread_by>0, sort=-h_notes_last_comment", "template=horse-note, $session->unread_by>0, sort=-h_notes_last_comment" ), $pageNum, $limit); But this is out of order: $notes = $paginator(array( "template=horse-note, h_notes_comments.count>0, h_notes_comments.$session->unread_by>0, sort=-h_notes_last_comment", "template=horse-note, $session->unread_by>0, sort=-h_notes_last_comment", "template=horse-note, sort=-h_notes_last_comment" ), $pageNum, $limit); Is there any reason you can think of that the third selector would interfere with the ordering? Link to comment Share on other sites More sharing options...
renobird Posted June 17, 2016 Share Posted June 17, 2016 Shot in the dark: are there duplicate pages being added by the last selector? (I'm really only just following from afar, I haven't like into how this works). Link to comment Share on other sites More sharing options...
LostKobrakai Posted June 17, 2016 Share Posted June 17, 2016 I'm just using import() to import the results of each "turn" into a single PageArray, therefore duplicates will be moved to the later places if they are returned twice. I can make some adjustments, so the class could actually prevent duplicates to be found again by later selectors. 4 Link to comment Share on other sites More sharing options...
Pete Jones Posted June 20, 2016 Author Share Posted June 20, 2016 Sounds like a good plan, thanks LK. Link to comment Share on other sites More sharing options...
LostKobrakai Posted June 20, 2016 Share Posted June 20, 2016 @Pete Jones Try this one, as it should prevent duplicate page selections by later selectors: https://github.com/LostKobrakai/Paginator/commit/e515bbba8a33be9259be810ff80ad2ad61078497 2 Link to comment Share on other sites More sharing options...
Pete Jones Posted June 21, 2016 Author Share Posted June 21, 2016 Okay, so I've now added the 'NoDuplicates' version into my page. I'm getting the same results as before, i.e. it's fine with the first 2 selectors, but the third one breaks it. include 'src/Paginator.php'; include 'src/PagesPaginator.php'; include 'src/PagesPaginatorNoDuplicates.php'; I feel it's 99% there though, and so much quicker than the previous query we were using. Link to comment Share on other sites More sharing options...
LostKobrakai Posted June 21, 2016 Share Posted June 21, 2016 Did you also change the part where you're instantiating the class? $paginator = new PagesPaginatorNoDuplicates(); If that's not the issue I might need to look more detailed into what's happening on your end. Link to comment Share on other sites More sharing options...
Pete Jones Posted June 21, 2016 Author Share Posted June 21, 2016 Erm, no. But only to check you were paying attention. I've done that and it now appears to be matching the order of the old query. Good work LK! Can we buy you a beer? One thing I do need to do now is to create pagination. If I'm now only returning the results I need for the current page, can I still work out how many pages I need in total? Link to comment Share on other sites More sharing options...
Recommended Posts
Create an account or sign in to comment
You need to be a member in order to leave a comment
Create an account
Sign up for a new account in our community. It's easy!
Register a new accountSign in
Already have an account? Sign in here.
Sign In Now