RFR: 8251850: Simplify ResourceMark constructors using delegation
Kim Barrett
kim.barrett at oracle.com
Mon Aug 17 02:24:10 UTC 2020
> On Aug 15, 2020, at 8:31 AM, Kim Barrett <kim.barrett at oracle.com> wrote:
>
>> On Aug 14, 2020, at 10:29 PM, 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
>
> I should have looked further along in resourceArea.hpp. All the
> changes I made to ResourceMark? DeoptResourceMark should have similar
> changes made to it.
>
> Well, I have a first round of comments from the ResourceMark changes. I can
> take those into account for DeoptResourceMark.
This ended up engendering a fairly substantial revision. I've taken at
least some of Thomas's comments into account along the way.
Changed ResourceMark and DeoptResourceMark to share ResourceMarkImpl, which
provides most of the implementation of both, eliminating the copy-paste-modify
"reuse" that used to be there.
Moved all of the state capture and rollback support into ResourceArea. The
mark objects include a captured state object and pass that back to the
ResourceArea when doing rollbacks.
Cleaned up the _nesting checking.
Removed nonsensical ResourceArea::_warned.
Maybe I should change the bug summary from
"Simplify ResourceMark constructors using delegation"
to something like
"Refactor ResourceMark and DeoptResourceMark for better code sharing"
Note: ResourceArea's handling of UseMallocOnly probably doesn't work in an
optimize build. ("probably" because I haven't actually tested it.) The
allocation side of it is #ifdef ASSERT, because functions it calls from
Arena are debug-only. But the free side is not conditionalized, so could be
used in any non-product build when the option is enabled. But maybe it's not
really a problem because the free side won't find any work. But this
conditionalization inconsistency is odd.
New webrevs:
full: https://cr.openjdk.java.net/~kbarrett/8251850/open.01/
incr: https://cr.openjdk.java.net/~kbarrett/8251850/open.01.inc/
Testing:
mach5 tier1-3
local hotspot:tier1 with -XX:+UseMallocOnly
local hotspot:tier1 with -XX:+ZapResourceArea
Note that compiler/loopopts/TestDeepGraphVerifyIterativeGVN.java times out
when run with -XX:+UseMallocOnly, with or without these changes.
More information about the hotspot-runtime-dev
mailing list