RFR: 8251850: Simplify ResourceMark constructors using delegation

[Thomas Stüfe]

Hi Kim,

This is a good cleanup. I looked over the old version and there are a
number of inconsistencies, most of which go away with this patch.

Remarks:

--

RM can be made immutable now. All members can be made const (also _thread
and _previous_resource_mark, if you reformulate the initializer a bit with
a '?' expression). As well as all member functions. AFAICS none of the
members get modified after construction, which makes sense considering what
RM is.

Some of the pointer members could be made destination-const too, at least
_previous_resource_mark. With RM being immutable there would be no reason
ever to have a non-const RM*. I know constness can snowball a bit, but I
think it's worth the work since it makes the code more clearer and prevents
accidents.

--

In constructor:

+    _area->_nesting++;
+    assert(_area->_nesting > 0, "must stack allocate RMs" );

Not your patch,but I do not understand this assert.

Is it - as the comment says - to make sure RMs are allocated on the stack,
as in StackObj? Would deriving from StackObj not be enough? How does this
assert help then?

Or is it to ensure that all allocations happen under a RM? That should be
checked in Arena::allocate(), and it is. Checking here is pointless since
we just established an RM, so there is nothing to check.

---

small nit: I like that you made protected private, now you could just omit
the qualifier altogether.

---

Since you are here ...

I'd prefer it if free_malloced_objects() would be DEBUG_ONLY() (so, not an
empty prototype but completely removed) which would be consistent with
other debug only code in this class. Side question, what is the difference
between ASSERT and !PRODUCT?

Thank you,

Thomas


On Sat, Aug 15, 2020 at 4:30 AM Kim Barrett <kim.barrett at oracle.com> wrote:

> Please review this simplification of the constructors for ResourceMark,
> accomplished using delegating constructors.
>
> Also made the constructors "explicit" and the class non-copyable.
>
> CR:
> https://bugs.openjdk.java.net/browse/JDK-8251850
>
> Webrev:
> https://cr.openjdk.java.net/~kbarrett/8251850/open.00/
>
> Testing:
> mach5 tier1
>
>