[m-rev.] for review: version_hash_table changes
Julien Fischer
jfischer at opturion.com
Mon May 6 15:39:43 AEST 2013
On Mon, 6 May 2013, Paul Bone wrote:
> On Mon, May 06, 2013 at 12:08:26PM +1000, Julien Fischer wrote:
>>
>> Hi Paul,
>>
>> On Fri, 3 May 2013, Paul Bone wrote:
>>
>>> diff --git a/library/version_hash_table.m b/library/version_hash_table.m
>>> index c624101..4e34f39 100644
>>> --- a/library/version_hash_table.m
>>> +++ b/library/version_hash_table.m
>>> @@ -112,6 +112,19 @@
>>> %
>>> :- func num_occupants(version_hash_table(K, V)) = int.
>>>
>>> + % Copy the hash table.
>>> + %
>>> + % Even though tables can be 'copied' by just creating a second reference
>>> + % to the same table this does not always allow programmers to reason
>>> + % about performance, as performance depends on how the two references
>>> + % are used after that point.
>>> + %
>>> + % Explicity copying the table with this function makes it easy to see at
>>> + % what points in a program the copying operation occurs, making it
>>> + % easier to reason about performance.
>>> + %
>>
>> I suggest simply:
>>
>> % Make a deep copy of the hash table.
>>
>
> It's not a deep copy, because it doesn't copy the items stored in the table.
That's true. (In retrospect I'm not sure array.copy, version_array.copy
etc were good names since builtin.copy/2 *does* do a deep copy.)
> I still wish to describe the performance implications as this is a case of a
> leaky abstraction.
I don't think it needs to be that wordy, most of this stuff about
performance implications is covered in the generic documentation for
version types (comments at the head of the version_array module).
Cheers,
Julien.
More information about the reviews
mailing list