On 9/18/20 3:43 AM, David Malcolm wrote:
On Thu, 2020-09-17 at 12:36 +0200, Aldy Hernandez via Gcc-patches
wrote:
This is the irange storage class. It is used to allocate the
minimum
amount of storage needed for a given irange. Storage is
automatically
freed at destruction.
It is meant for long term storage, as opposed to int_range_max which
is
meant for intermediate temporary results on the stack.
The general gist is:
irange_pool pool;
// Allocate an irange of 5 sub-ranges.
irange *p = pool.allocate (5);
// Allocate an irange of 3 sub-ranges.
irange *q = pool.allocate (3);
// Allocate an irange with as many sub-ranges as are currently
// used in "some_other_range".
irange *r = pool.allocate (some_other_range);
FWIW my first thoughts reading this example were - "how do I deallocate
these iranges?" and "do I need to call pool.deallocate on them, or is
that done for me by the irange dtor?"
Thus the description front and center of the header file:
"Storage is automatically freed at destruction..."
I think of a "pool allocator" as something that makes a small number of
large allocation under the covers, and then uses that to serve large
numbers of fixed sized small allocations and deallocations with O(1)
using a free list.
Ah, I didn't know pool had a different meaning.
[...]
+// This is the irange storage class. It is used to allocate the
+// minimum amount of storage needed for a given irange. Storage is
+// automatically freed at destruction.
"at destruction" of what object - the irange or the irange_pool?
Reading the code, it turns out to be "at destruction of the
irange_pool", and it turns out that irange_pool is an obstack under the
covers (also called a "bump allocator") and thus, I believe, the
lifetime of the irange instances is that of the storage instance.
The sentence is talking about the storage class, so I thought it was
obvious that the destruction we talk about is the storage class itself.
I suppose if it isn't clear we could say:
"Storage is automatically freed at destruction of the storage class."
I think it would be clearer to name this "irange_obstack", or somesuch.
I'd prefer something more generic. We don't want to tie the name of the
allocator to the underlying implementation. What if we later change to
malloc? We'd have to change the name to irange_malloc.
irange_allocator? Or is there something more generically appropriate here?
+//
+// It is meant for long term storage, as opposed to int_range_max
+// which is meant for intermediate temporary results on the stack.
+
+class irange_pool
+{
+public:
+ irange_pool ();
+ ~irange_pool ();
+ // Return a new range with NUM_PAIRS.
+ irange *allocate (unsigned num_pairs);
+ // Return a copy of SRC with the minimum amount of sub-ranges
needed
+ // to represent it.
+ irange *allocate (const irange &src);
+private:
+ struct obstack irange_obstack;
...and thus to rename this field to "m_obstack" or similar.
Will do.
Thanks.
Aldy
