Re: /bzr/squid3/trunk/ r10914: Added InstanceId to uniquely identify concurrent messages, transactions, etc.

On 10/05/2010 03:43 AM, Kinkie wrote:

> I'm glad to see that you got around to implementing this - I had
> toyed with it myself, but with inferior results.

I hope you can use InstanceId instead of "this" for SBuf/MemBlob
identification.

>> + */
>> +template<class Class>
>> +class InstanceId
>> +{
>> +public:
>> + typedef unsigned int Value; ///< id storage type; \todo: parameterize?
>
> Why not use uint32_t? It's defined, and it gives consistent wrapping results

Either should work. Int just seemed overall simpler to me. If others
think we should change Value type to uint32_t, I would not be against it.

>> +
>> + InstanceId(): value(++Last ? Last : ++Last) {}
>
> It seems that you don't like object-id 0. Is there any particular
> reason for this?

I want to use this class to identify messages and match requests with
replies. When there is no pending message/transaction, it is handy to
use zero for the ID.

>> +
>> + operator Value() const { return value; }
>> + bool operator ==(const InstanceId&o) const { return value == o.value; }
>> + bool operator !=(const InstanceId&o) const { return !(*this == o); }
>
> While these don't hurt, I can't really understand where they could be
> useful. Are they?

The top conversion is for storing IDs in messages and such. It may be
also handy for ID-based math (e.g., measuring the distance between two
messages). However, I am not sure it was a good idea to include this
conversion because it may be too risky in general. We may want to try to
remove it and see what happens.

The two equality operators are for matching requests with replies. One
interesting aspect here is that we currently restrict comparisons to IDs
in the same category. We may also add a "less than" operator for
ordering/searching messages in STL containers.

I suspect the API will change as we start using IDs for more purposes,
in more contexts. The above is just a start.

Cheers,

Alex.
Received on Tue Oct 05 2010 - 16:32:03 MDT