From mboxrd@z Thu Jan  1 00:00:00 1970
From: wmorgan-sup@masanjin.net (William Morgan)
Date: Sat, 22 Aug 2009 07:10:04 -0700
Subject: [sup-talk] [PATCH] cache results of Person.from_address
In-Reply-To: <1250491172-19317-1-git-send-email-rlane@club.cc.cmu.edu>
References: <1250491172-19317-1-git-send-email-rlane@club.cc.cmu.edu>
Message-ID: <1250949615-sup-1773@masanjin.net>

This looks good. Two minor questions before I apply:

Reformatted excerpts from Rich Lane's message of 2009-08-16:
> The regexes in this function are very expensive, so caching improves
> performance significantly for queries and slightly for indexing.

When you say this affects query performance, is it just the contact-list
query, or is there some other mechanism by which this is slowing down
regular queries?

Also in this method:

> +  def []=(k,v)
> +    if @values.size < @n
> +      @values[k] = v
> +      @marks[k] = @i
> +    else
> +      if @delete_stack.empty?
> +        sweep
> +      else
> +        k2 = @delete_stack.pop
> +        @values.delete k2
> +        @marks.delete k2
> +        self[k] = v
> +      end
> +    end
> +  end

Wouldn't it be better to do this?

       else
         if @delete_stack.empty?
           sweep
         end

         unless @delete_stack.empty?
           k2 = @delete_stack.pop
           @values.delete k2
           @marks.delete k2
           self[k] = v
         end

So we check the delete stack even after calling sweep, and we allow for
the value to be nil.

Thanks!
-- 
William <wmorgan-sup at masanjin.net>