Skip to content

Commit

Permalink
8314935: Shenandoah: Unable to throw OOME on back-to-back Full GCs
Browse files Browse the repository at this point in the history
Reviewed-by: kdnilsen, ysr
  • Loading branch information
William Kemper authored and Y. Srinivas Ramakrishna committed Sep 7, 2023
1 parent 4c6d7fc commit 716201c
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 17 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,8 @@ class ShenandoahCollectorPolicy : public CHeapObj<mtGC> {
private:
size_t _success_concurrent_gcs;
size_t _success_degenerated_gcs;
size_t _success_full_gcs;
// Written by control thread, read by mutators
volatile size_t _success_full_gcs;
size_t _alloc_failure_degenerated;
size_t _alloc_failure_degenerated_upgrade_to_full;
size_t _alloc_failure_full;
Expand Down Expand Up @@ -82,6 +83,10 @@ class ShenandoahCollectorPolicy : public CHeapObj<mtGC> {
size_t cycle_counter() const;

void print_gc_stats(outputStream* out) const;

size_t full_gc_count() const {
return _success_full_gcs + _alloc_failure_degenerated_upgrade_to_full;
}
};

#endif // SHARE_GC_SHENANDOAH_SHENANDOAHCOLLECTORPOLICY_HPP
21 changes: 5 additions & 16 deletions src/hotspot/share/gc/shenandoah/shenandoahHeap.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -878,25 +878,14 @@ HeapWord* ShenandoahHeap::allocate_memory(ShenandoahAllocRequest& req) {
// It might happen that one of the threads requesting allocation would unblock
// way later after GC happened, only to fail the second allocation, because
// other threads have already depleted the free storage. In this case, a better
// strategy is to try again, as long as GC makes progress.
//
// Then, we need to make sure the allocation was retried after at least one
// Full GC, which means we want to try more than ShenandoahFullGCThreshold times.

size_t tries = 0;

while (result == nullptr && _progress_last_gc.is_set()) {
tries++;
control_thread()->handle_alloc_failure(req);
result = allocate_memory_under_lock(req, in_new_region);
}

while (result == nullptr && tries <= ShenandoahFullGCThreshold) {
tries++;
// strategy is to try again, as long as GC makes progress (or until at least
// one full GC has completed).
size_t original_count = shenandoah_policy()->full_gc_count();
while (result == nullptr
&& (_progress_last_gc.is_set() || original_count == shenandoah_policy()->full_gc_count())) {
control_thread()->handle_alloc_failure(req);
result = allocate_memory_under_lock(req, in_new_region);
}

} else {
assert(req.is_gc_alloc(), "Can only accept GC allocs here");
result = allocate_memory_under_lock(req, in_new_region);
Expand Down

5 comments on commit 716201c

@openjdk-notifier
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@earthling-amzn
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/backport openjdk/jdk17u-dev

@openjdk
Copy link

@openjdk openjdk bot commented on 716201c Sep 7, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@earthling-amzn Could not automatically backport 716201c7 to openjdk/jdk17u-dev due to conflicts in the following files:

  • src/hotspot/share/gc/shenandoah/shenandoahHeap.cpp

Please fetch the appropriate branch/commit and manually resolve these conflicts by using the following commands in your personal fork of openjdk/jdk17u-dev. Note: these commands are just some suggestions and you can use other equivalent commands you know.

# Fetch the up-to-date version of the target branch
$ git fetch --no-tags https://git.openjdk.org/jdk17u-dev.git master:master

# Check out the target branch and create your own branch to backport
$ git checkout master
$ git checkout -b earthling-amzn-backport-716201c7

# Fetch the commit you want to backport
$ git fetch --no-tags https://git.openjdk.org/jdk.git 716201c77d160dc78db61957aa002eef71641688

# Backport the commit
$ git cherry-pick --no-commit 716201c77d160dc78db61957aa002eef71641688
# Resolve conflicts now

# Commit the files you have modified
$ git add files/with/resolved/conflicts
$ git commit -m 'Backport 716201c77d160dc78db61957aa002eef71641688'

Once you have resolved the conflicts as explained above continue with creating a pull request towards the openjdk/jdk17u-dev with the title Backport 716201c77d160dc78db61957aa002eef71641688.

@earthling-amzn
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/backport openjdk/jdk21u

@openjdk
Copy link

@openjdk openjdk bot commented on 716201c Sep 8, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@earthling-amzn the backport was successfully created on the branch earthling-amzn-backport-716201c7 in my personal fork of openjdk/jdk21u. To create a pull request with this backport targeting openjdk/jdk21u:master, just click the following link:

➡️ Create pull request

The title of the pull request is automatically filled in correctly and below you find a suggestion for the pull request body:

Hi all,

This pull request contains a backport of commit 716201c7 from the openjdk/jdk repository.

The commit being backported was authored by William Kemper on 7 Sep 2023 and was reviewed by Kelvin Nilsen and Y. Srinivas Ramakrishna.

Thanks!

If you need to update the source branch of the pull then run the following commands in a local clone of your personal fork of openjdk/jdk21u:

$ git fetch https://github.com/openjdk-bots/jdk21u.git earthling-amzn-backport-716201c7:earthling-amzn-backport-716201c7
$ git checkout earthling-amzn-backport-716201c7
# make changes
$ git add paths/to/changed/files
$ git commit --message 'Describe additional changes made'
$ git push https://github.com/openjdk-bots/jdk21u.git earthling-amzn-backport-716201c7

Please sign in to comment.