Committed, thanks.

Just rolled this code to production today and found a couple minor 
bugs (php notices).  Here's a patch against HEAD to make it a little 
safer.

Committed, thanks!

Here's a new patch against a fresh copy of imsp.php.  There was no 
reason for array_push, so I changed it to use $id[] = $id[$i] as 
suggested.



The only benchmarking we did was for users w/o many groups, it showed 
no change in performance for such cases. Benchmarking for books with 
many groups still needs to be done.

I can't get this patch to apply cleanly to HEAD.   Can you please 
create a new patch against a freshly updated copy of imsp.php?



Also, is there a specific reason you choose to use array_push() to add 
a single value to the end of the $id  array?  Using $id[] = $id[$i] 
would be slightly more efficient, as it avoids the overhead of a 
function call.



Just curious, did you do any benchmarking to see what the improvement 
in performance was?



Thanks!

For the reasons discussed in Ticket 855, essentially that everything 
is loaded and filtered "client-side" (php layer) when browsing, some 
kind of speedup strategy is needed. In particular, the way Turba reads 
Groups seems very inefficient when loading everything for browsing. It 
ends up searching and fetching each email address  in the Group, even 
though it's also searching and fetching those contacts just as a part 
of the address book being browsed.



Here's a strategy to reduce some of the redundant IMSP fetching:

1. _search sends all ids in the addressbook to _read. (As per Ticket 4881)

2. While stepping through the id list, if a Group is encountered, push 
the group's id to the end of the list (we'll handle Groups when all 
contacts have been loaded).

3. Because the Group had to be read to be identified as a group, it is 
stored temporarily before proceeding.

4. After all contacts are loaded, the Groups are processed. Each group 
is loaded from the temporary array at this point, then it looks 
through the existing contact results for a match instead of going back 
to the IMSP server. If no result is found, it falls back to searching 
the IMSP server.



NOTE: This strategy requires the patches from Ticket 4881, as all the 
optimizations take place in the _read function.