[MERGE] Factor out the Graph.heads() cache from _RevisionTextVersionCache for reuse, and use it in commit.

[John Arbash Meinel]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Andrew Bennetts wrote:
> Robert Collins wrote:
>> On Wed, 2007-10-17 at 21:51 -0500, John Arbash Meinel wrote:
> [...]
>>> I would argue that you let them create their own sets. Rather than forcibly
>>> creating a new object every time you enter this function.
>>>
>>> If *all* callers need to modify the set, then fine. But I doubt that is the case.
>>>
>>> Maybe it is an API break... but probably a rather minor one at that.
>> Well the cache is new, but it seems like a good idea to me to have its
>> api match that of the function it wraps, so that the cache can be
>> inserted and removed seamlessly.
> 
> I think it's good to have an API that's safe by default (and just like the API
> it is a cache for).  If we make it easy to corrupt the cache we're inviting
> confusing bugs down the track.  
> 
> So, I think returning a copy is fine, just add a brief comment in the code to
> make it clear that making a copy is the intent, so that future reviewers don't
> look at the code and think “huh, that's odd”.
> 
> Caching and returning a frozenset would be just as fine from a safety point of
> view, so I'd say Robert should just do whichever way is more convenient for him.
> 
> -Andrew.

I'm okay with this. Considering this was implemented because it was seen as a
performance bottleneck we may want to try to avoid creating a new object for
each call.

Anyway, I do understand why he's doing this, so it can go in as is.

John
=:->

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.7 (Darwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFHGBsxJdeBCYSNAAMRAoFtAKDYJLyXiQf7+hpuewj9EGkeWtofgwCgzYp3
qyWcVtti2QPeW7151eiSdHU=
=rtqM
-----END PGP SIGNATURE-----