RFR: 8245203/8245204/8245208: ZGC: Don't hold the ZPageAllocator lock while committing/uncommitting memory
Stefan Karlsson
stefan.karlsson at oracle.com
Tue May 19 09:59:07 UTC 2020
On 2020-05-18 23:23, Per Liden wrote:
> Please review this series of three patches to rework the page
> allocation path so that we don't hold the ZPageAllocator lock while
> committing/uncommitting memory. Patch 1 & 2 are small and preparatory.
> Patch 3 is the main patch and it's unfortunately fairly large as it
> was hard to break up in a sensible way.
>
...
>
> 2) 8245204: ZGC: Introduce ZListRemoveIterator
>
> Add ZListRemoveIterator, which unlike ZListIterator, iterates over a
> non-const "ZList<T>" and each call to "next()" also removes the
> returned element from the list.
>
> Bug: https://bugs.openjdk.java.net/browse/JDK-8245204
> Webrev: http://cr.openjdk.java.net/~pliden/8245204/webrev.0
I find it a bit odd that the iterator removes an element in the
constructor. Would it be possible to get rid of the _next field, and
change the next(...) function to do:
if (!_list->is_empty()) {
*elem = Forward ? _list->remove_first() : _list->remove_last(); return true;
}
return false;
or, since remove_first/last already checks is_empty:
*elem = Forward ? _list->remove_first() : _list->remove_last(); return
*elem != NULL;
One could argue if there's a real need for the iterator. This
+ ZListRemoveIterator<ZPage> iter(&pages);
+ for (ZPage* page; iter.next(&page);) {
could simply be:
for (ZPage* page; page = pages.remove_first();) {
but I'm fine with an iterator if you like that.
Thanks,
StefanK
More information about the hotspot-gc-dev
mailing list