[dev] Horde_Share_Base::listShares()

Fri Jan 14 18:17:07 UTC 2011

Zitat von Michael Rubinsky <mrubinsk at horde.org>:

> Quoting Jan Schneider <jan at horde.org>:
>
>> Zitat von Gunnar Wrobel <wrobel at horde.org>:
>>
>>> Hi!
>>>
>>> The function mentioned above does support several sorting/paging  
>>> features ('from', 'count', 'sort_by', 'direction'). As far as I  
>>> can tell none of these parameters are actively used within our  
>>> code base (with the exception of 'sort_by' but 'name' seems to be  
>>> the only setting used here). Currently only the Sql-Drivers  
>>> supports these features and I wonder if we really need them.
>
> Huh. That's embarrasing. I'm pretty sure it worked at one point, but  
> maybe that was back in the DT days. I never realised that the paging  
> parameters were ignored by the SQL share drivers.  I know that Ansel  
> passes these values when performing certain searches, like listing  
> all of a user's galleries, or most recent galleries, or any number  
> of other searches. The View code in Ansel, however, also honors the  
> per-page prefs, which is probably why I never noticed this.
>
> Ansel could potentially generate a large number of shares, since  
> each gallery/subgallery is a separate share - though I imagine it  
> would be rare to reach the thousands per user number. I can  
> understand the issue of large session caches in certain cases.
>
> I have mixed feelings about not implementing this though. On the one  
> hand, if the user *does* have a large number of shares - and I'm  
> honestly not sure how large that number would have to be to start  
> beccoming an issue - then it seems a waste to carry around  
> potentially the user's entire share list in the session if all that  
> was ever needed were the first 9 shares. For example - imagine I  
> have 100 Ansel galleries. I log into Horde, and since Ansel is my  
> initial application, I load the first page of my gallery list, I  
> then move on to reading email and don't go back to Ansel again  
> during this session.  Without support for paging, this would cause  
> all 100 of my ansel shares to be carried around for the life of my  
> session...and from what you are saying this is currently happening.
>
> So I guess the question is: Is it more expensive to keep more search  
> result entries in the list cache - each with a smaller subset of  
> shares, or to keep a smaller number of search results, but each with  
> larger numbers of shares?
>
> I think the case where a user would load thousands of shares, then  
> sort them in multiple ways is a fringe case. More likely, a smaller  
> subset of those shares would be loaded at a time e.g., when paging  
> through a gallery list.
>
> Another option is to ignore the paging/sorting parameters when  
> caching and hitting the DB, and apply to paging limits in PHP, this  
> has the disadvantage of having to apply the sort to the share list  
> each time, prior to applying the limits. Since these parameters are  
> not currently being passed to the DB anyway, this is probably what  
> we are already doing in the application code.
>
>>> Horde_Share_Base::listShares() is fully replaced within  
>>> Horde_Share_Sql and Horde_Share_Sql_Hierarchical.
>
> Doing it this way is probably historical cruft, left over from when  
> these drivers were written, and DT was the only supported driver.

No, this is exactly for the paging, because overwriting the default  
method in the base class (which should do the paging/sorting in PHP  
code) allows to implement optimized paging/sorting in the backend.
It's well possible that this doesn't work anymore, but I'm pretty sure  
this was the original idea behind it.

>>> For Horde_Share_Kolab and Horde_Share_Datatree  
>>> Horde_Share_Base::listShares() is called which delegates to  
>>> Horde_Share_{Datatree,Kolab}::_listShares(). The sorting/paging  
>>> apparently should be handled within Horde_Share_Base::listShares()  
>>> for these drivers. The current implementation is incomplete however.
>
> Now that DT is deprecated, if we decide to implement the paging in  
> the DB, we could probably just make Horde_Share_Base::listShares()  
> abstract. Otherwise, yeah, we could just implement it in  
> Horde_Share_Base::listShares()
>
>>> I don't mind completing the implementation if necessary but there  
>>> are some things that currently puzzle me. Assuming a user has a  
>>> few thousand shares and we would use the paging feature for  
>>> example: Wouldn't the current Horde_Share_Sql:listShares() quickly  
>>> fill the Horde_Share_Base::_listcache with plenty of caching  
>>> information that gets stored in the session? For every sort  
>>> setting it would be the same thing.
>
> This is assuming that all thousand shares would be returned in each  
> query. If we implement paging, it's likely only a small subset of  
> those shares would be returned for any one query - the amount that  
> would fit on a page. The exception to this is when querying for  
> things like select lists, but those would likely not be requested  
> with more than a single sort parameter.  Generally, though, yes,  
> each sort/page combmination would be cached separately.
>
>>> I also wonder a bit if that is actually information that should go  
>>> into the session at all.
>
> I know that in FW_3, at least at one time, it greatly improved  
> performance since share queries tend to be expensive, especially on  
> mysql. Jan could probably answer this part better than I, since he  
> has done some benchmarking recently.

Yes, it improves performance a lot. And IIRC Michael has already  
reduced session storage a lot by implementing Serializable.
It might still offer enough benefit if this cached in a Horde_Cache  
backend instead of the session though. This would be separate from  
existing share caching though.

>>> On the other hand there has been a lot of work put into the Share  
>>> driver recently and as far as I understand it there is more to  
>>> come. I just wanted to get some feedback to understand where this  
>>> is heading and if I should ignore the topic for now.
>>
>> If we don't use some of the paging/sorting features and don't come  
>> up with any use case for it (and honestly, who is going to have  
>> such a huge load of shares?), I'm fine with removing them to reduce  
>> complexity.
>
> Honestly not 100% sure which way I'd prefer, but I think I'm leaning  
> towards having the DB honor the parameters, instead of having to  
> sort/page only in pure php.
>
>
> Thanks,
> mike
>
> --
> The Horde Project (www.horde.org)
> mrubinsk at horde.org
>
> "Time just hates me. That's why it made me an adult." - Josh Joplin
> -- 
> Horde developers mailing list
> Frequently Asked Questions: http://horde.org/faq/
> To unsubscribe, mail: dev-unsubscribe at lists.horde.org
>

Jan.

-- 
Do you need professional PHP or Horde consulting?
http://horde.org/consulting/