[Webkit-unassigned] [Bug 67823] [NRWT] Provides a simple LRU cache class in Python.

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=67823





--- Comment #17 from Hayato Ito <hayato at chromium.org>  2011-09-20 01:10:42 PST ---
Hi Tony, thank you for the review.

Since Ai-san, the author of the patch, has already left Google, I'll update the patch later, addressing your comment.

(In reply to comment #16)
> (From update of attachment 107615 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=107615&action=review
> 
> FWIW, I think this would be less confusing if you just kept a list of keys rather than the linked list.  As long as the capacity is small (maybe less than 20 entries), it'll still be plenty fast to update.  But this approach is fine too if you fix the memory leak I mentioned.
> 
> > Tools/Scripts/webkitpy/common/lru_cache.py:39
> > +    Args:
> > +        capacity: a cache memory capacity
> 
> Nit: This should be in the docstring for __init__.
> 
> > Tools/Scripts/webkitpy/common/lru_cache.py:57
> > +            if self._capacity == 1:
> > +                self._one_node(key, value)
> 
> I think you need to clear self._first here or the new node will point to the old value.  If the capacity is 1, we leak all the old objects because we hold a ref to them.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.