8360817: [ubsan] zDirector select_worker_threads - outside the range of representable values issue

[MBaesken]

When running the jtreg test
gc/z/TestMappedCacheHarvest.java
with ubsan-enabled binaries on macOS aarch64, the following ubsan error is reported :

 stderr: [/jdk/src/hotspot/share/gc/z/zDirector.cpp:715:33: runtime error: 6.1962e+10 is outside the range of representable values of type 'unsigned int'
    #0 0x109e368fc in select_worker_threads(ZDirectorStats const&, unsigned int, ZWorkerSelectionType) zDirector.cpp:715
    #1 0x109e3658c in initial_workers(ZDirectorStats const&, ZWorkerSelectionType) zDirector.cpp:804
    #2 0x109e35df0 in ZDirector::run_thread() zDirector.cpp:932
    #3 0x109ecd5f4 in ZThread::run_service() zThread.cpp:28
    #4 0x108cd1e50 in ConcurrentGCThread::run() concurrentGCThread.cpp:47
    #5 0x109ce770c in Thread::call_run() thread.cpp:243
    #6 0x10985be28 in thread_native_entry(Thread*) os_bsd.cpp:599
    #7 0x19fa8ef90 in _pthread_start+0x84 (libsystem_pthread.dylib:arm64e+0x6f90)
    #8 0x19fa89d30 in thread_start+0x4 (libsystem_pthread.dylib:arm64e+0x1d30)

After I added a bit of tracing to select_worker_threads it seems we get WAY too high young_to_old_ratio values; in my locally failing example
young_to_old_ratio:27561965412.478878
this leads to values out of range of uint and that makes ubsan complain about the uint cast
uint(young_workers * young_to_old_ratio) .
We clamp the value anyway to the range 1u to ZOldGCThreads ; so it probably makes sense to avoid such high factors for young_to_old_ratio .

---

Progress

• Change must be properly reviewed (1 review required, with at least 1 Reviewer)
• Change must not contain extraneous whitespace
• Commit message must refer to an issue

Issue

• JDK-8360817: [ubsan] zDirector select_worker_threads - outside the range of representable values issue (Bug - P4)

Reviewers

• Lutz Schmidt (@RealLucy - Reviewer)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/26186/head:pull/26186
$ git checkout pull/26186

Update a local copy of the PR:
$ git checkout pull/26186
$ git pull https://git.openjdk.org/jdk.git pull/26186/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 26186

View PR using the GUI difftool:
$ git pr show -t 26186

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/26186.diff

Using Webrev

Link to Webrev Comment

[bridgekeeper]

👋 Welcome back mbaesken! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

[openjdk]

@MBaesken This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8360817: [ubsan] zDirector select_worker_threads - outside the range of representable values issue

Reviewed-by: lucy

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 23 new commits pushed to the master branch:

• a9bd1ad: 8361520: Stabilize SystemGC benchmarks
• b1fa1ec: 8360707: Globally enumerate all blobs, stubs and entries
• 83feb7a: 8361599: [PPC64] enable missing tests via jtreg requires
• ... and 20 more: https://git.openjdk.org/jdk/compare/2349304bb108adb0d5d095e8212d36d99132b6bb...master

As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.

[openjdk]

@MBaesken The following label will be automatically applied to this pull request:

• hotspot-gc

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command.

[mlbridge]

Webrevs

• 00: Full (1cdc7b9f)

[RealLucy]

LGTM - as local remedy.
I would feel better though if the root cause of the out-of-bounds value would be understood.

[MBaesken]

calculate_young_to_old_worker_ratio returns a double and this can get (in theory) very large.
and the calculation part of this method

  const double old_vs_young_efficiency_ratio = current_old_bytes_freed_per_gc_time / current_young_bytes_freed_per_gc_time;
  return old_vs_young_efficiency_ratio;

can , depending on what has been freed, get also in practice very large; so it is not really a surprise that we see sometimes large values.

[MBaesken]

Thanks for the review Lucy !
May I get a second review ?