Code review comment for lp:~seif/zeitgeist/add_cache_for_get_events

Revision history for this message
Seif Lotfy (seif) wrote :

I did some testing an with a cache of 700 events my solution starts becoming
slower thus I removed it for a more consitent lru cache (the old one). This
removes 50% off the get_events

On Tue, Dec 7, 2010 at 8:01 PM, Mikkel Kamstrup Erlandsen <
<email address hidden>> wrote:

> Review: Needs Fixing
> In lrucache.py:
> __delitem__ must also update id_seq
>
> Also can LRUCache maybe log a warning if you create it with maxsize > 1000
> or something. This implementation scales very badly, although as you say, it
> may well be faster for small caches.
>
> Lastly, have you actually benchmarked this with 1000 items in the cache?
> Because my guess says that that should be around the tipping point where the
> linear searches starts to make an impact...
>
> Maybe reduce the cache size to 500 just to feel a little safer?
> --
>
> https://code.launchpad.net/~seif/zeitgeist/add_cache_for_get_events/+merge/42327
> You are the owner of lp:~seif/zeitgeist/add_cache_for_get_events.
>

--
This is me doing some advertisement for my blog http://seilo.geekyogre.com

« Back to merge proposal