[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