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