Re: [boost] [Boost-announce] Review of Intrusive library begins today

From: Dave Harris (brangdon_at_[hidden])
Date: 2007-03-20 09:19:39

In-Reply-To: <45F5011E.8484B515_at_[hidden]>
joaquin_at_[hidden] wrote (abridged):
> The review of Ion Gaztañagas's Intrusive library begins today

I have spent half an hour reading the documentation. Overall I like it. I  
have some questions and comments.

Is there any reason why Destroyers must destroy? It seems to me you could 
have a Destroyer which pushed the value onto another list. Is that right? 
If so, is it a reasonable thing to do? If so, perhaps "Destroyer" is not 
the best name. I'd suggest "Sink" instead.

Could remove_and_destroy take a default Destroyer argument that deleted 
the value?

I hate the name "current" for the function that turns a value into an 
iterator. I'd prefer "to_iterator" or "as_iterator" or similar. "Current" 
just doesn't seem relevant to what it does, and the core thing it does is 
not indicated by that name.

Did you consider having pop_back() return a reference to the value popped? 
As I understand it, the usual reasons why this a bad thing don't apply 
here; returning a reference cannot throw, and the value referred to isn't 
deleted.

Did you consider making it easier to use as a replacement for list<value*> 
? The semantics are already close, but the intrusive lists use references 
where the non-intrusive ones use pointers. I can see me having to edit out 
a lot of asterixes in my existing code.

I was going to ask whether it would have been a good idea to allow a value 
type to be in several lists of the same type, but the more I think about 
it the more I agree that requiring the lists to be different types is 
best.

I agree with Tom Brinkman that the introduction page of the documentation 
could do more to motivate the library. The core idea of using memory in 
the value type to store container-specific data, which is what makes the 
container "intrusive", needs to be mentioned earlier.

I was pleased to see you mention the typical size overheads for values and 
containers. It seemed to me that the overhead for containers was a bit 
buried. I'd recommend you draw up a little comparison table which listed 
typical overheads for each type of container - hopefully including a good 
implementation of std containers if that's not too controversial.

I would also like to see some speed measurements comparing std with 
intrusive lists and vectors. Obviously user's milage will vary, but given 
that performance is a major motivation for using this library I'd like to 
see at least some empirical indication that it can be faster.

Regardless of the above I vote that Intrusive be accepted as a boost 
library.

I am fairly knowledgable of the domain, having spent a lot of time using 
intrusive lists in C 20 years ago. My current app has some rigid 
data-structures currently based on std::list which could probably benefit 
from intrusive lists, but I'm not currently in a position to try this so 
the above is based only on reading the online documentation.

> To: boost_at_[hidden], boost-users_at_[hidden],
>        boost-announce_at_[hidden]
> Reply-To: boost-announce_at_[hidden]

Incidently, I tried to send this message by replying, and my mail software 
used the Reply-To: field and sent it to boost-announce, where it was 
(unsurprisingly) rejected. Maybe Reply-To: could be set to 
boost_at_[hidden] instead?

-- Dave Harris, Nottingham, UK.