On 09/08/2011 01:09 AM, Christophe wrote:
Timon Gehr , dans le message (digitalmars.D:144133), a écrit :
On 09/07/2011 11:43 PM, Christophe wrote:
Ok, don't drop this feature then. But the API is too complicated. I
think you can keep the same functionality with an API that is much
easier to understand:
- Make RegionAllocatorStack private. This is implementation detail no
one has to know about.
- Create a method for RegionAllocator that is called something like
"invalidatingCopy", which is the same as creating a new regionAllocator
with the same regionAllocatorStack. Tell about the fact that the
invalidatingCopy make the calls to the RegionAllocator being copied
throw until all instances of the invalidatingCopy go out of scope.
- Make a function to call "invalidatingCopy" on the default thread-local
RegionAllocator, to replace newRegionAllocator, but I insist, please use
a different name since it is confusing: it makes you think the allocator
is breand new, whereas it reuses a common stack.
It is a brand new allocator which operates on an existing stack.
Well, then a simple copy of a RegionAllocator is also brand new, isn't
it ? As is a copy of a File, etc.
- Make a function to create a brand new allocator with its own stack,
and specifies that the previous function may be more efficient.
With this, I think everybody will understand easily what is an
invalidatingCopy of your allocator. You can explain in allocate,
free and other methods that you check for the existence of an
invalidatingCopy without having to tell anything about
sharing RegionAllocatorStack with other RegionAllocators.
Actually, I think the current semantics are a lot less confusing than
the invalidatingCopy proposal:
alloc2 = alloc1.invalidatingCopy();
alloc3 = alloc1.invalidatingCopy();
now alloc2 is invalid, even though you called the invalidatingCopies on
alloc1 only. To get why this is the case, the programmer has to
understand that there is an underlying stack structure, confusingly
hidden away. Therefore, hiding the RegionAllocStack is a leaky
abstraction and does not help.
You are right, this is a problem. The documentation of invalidatingCopy
should tell that previously created invalidatingCopies are also
invalidated, and that they should be destroyed in the right order. The
documentation could (and should) tell about the fact that an internal
stack is shared between invalidating copy, that is not the point.
This is the same explanations that what you have to explain carefully in
dsimcha's API.
The point is that you gave an explicit name to invalidatingCopy. With
Basically, what you are trying to do is hiding the underlying reason for
why the function behaves in a way that in your eyes has to result in an
explicit name for it. There are no 'invalidating' semantics. There are
*stack* semantics, and removing the part of the API that is explicit
about that (RegionAllocStack) is not going to help.
this API, a user can start to use a RegionAllocator easily, without
knowing about the possibility of the powerful but dangerous idea sharing
stacks. But then, this user see or type this method called
invalidatingCopy, the "invalidating" in the name sounds dangerous and
rings an alarm bell, and he reads carefully the documentation of
invalidatingCopy. All information about stack sharing is gathered in the
documentation of invalidatingCopy. Now, the user may remember having
read in the documentation of allocate, free, etc, that these methods
could throw an exception if an invalidatingCopy had been created, but
having also decided not to use this dangeroursly looking
invalidatingCopy. When he reads the documentation of allocate again, he
now understand quite well what is an invalidating copy.
Well, I don't really want dangerously looking stuff in my codebase, but
I definitely am going to use the region allocator. BTW invalidatingCopy
is a bad name because:
1. nothing is invalidated, the other allocators are just 'suspended'.
2. nothing is copied, the result is a new region allocator.
It is just like a function that is lower on the call stack cannot make
any stack allocations until the function it has called returns.
With dsimcha's API, the user want a RegionAllocator, and types
newRegionAllocator. When he wants a second RegionAllocator, he calls
newRegionAllocator again, but still think he can use the first
allocator. Nothing let him think calling this method twice was so
dangerous. The error message make him look at the documentation of
RegionAllocator.free (if it is the method that threw). He looks at that,
gets confused, does not manage to understand that "an instance of
RegionAllocator using the same RegionAllocatorStack" is something
different than a simple copy of the RegionAllocator structure. He tries
to read a bit further but rails at the documentation being spread
between the introduction of the package, RegionAllocator,
RegionAllocatorStack, and newRegionAllocator, and swears he will not use
RegionAllocator again...
This is biased in that it assumes that the documentation will magically
be more understandable if the API changes from a (imo) comprehensive and
powerful one to a dangerously-looking one.
I do not think an user who does not understand the region allocator will
make effective use of it anyways. But I agree that maybe the
documentation should provide some more examples how to make effective
use of the RegionAllocator. Eg. at synopsis, currently a part of the
allocator interface is presented. Probably it would be better to have
synopsis before the comparisons to call stack and heap and to have it
use a larger part of the API that is exclusive to RegionAlloc.
I, as a "new user", understood how RegionAllocator worked only because I
wanted to help in the review process by trying to find out what went
wrong. And I really thought simple copies of a RegionAllocator would
invalidate each other because they used the same RegionAllocatorStack,
until I went down in the code to see how the correctRegionIndex field
worked.
It even hurts, because with it you cannot
pass around a RegionAllocStack without a RegionAlloc operating on it.
How is this a problem ?
Creating a new RegionAlloc is not free. I don't want overhead for stuff
I don't need.
If it really is, you could still make RegionAllocatorStack available,
but just further down in the documentation, so the user do not have to
read it all if he do not want to use it.
Again, making the stack semantics obscure by trying to not show it to
the user as good as possible is not going to help anyone understanding
the stack semantics.
