RFR: 8335907: JFR: Make SettingControls more robust

Erik Gahlin egahlin at openjdk.org
Thu Jul 25 20:50:53 UTC 2024


Could I have review of changes to the SettingControl implementations to make them more robust:

- Consistently ignore invalid values for the methods `combine(Set)` and `setValue(String)`.
- Consistently set the textual value field in the `SettingControl` after the actual value has been changed.
- Consistently set the textual value field to the default value in the constructor.

A test was added to ensure that current and future setting controls are properly tested. This also improves test coverage. See [JDK-8309052](https://bugs.openjdk.org/browse/JDK-8309052)

**Implementation comments:**

- A `jdk.jfr.internal.util.Rate` class was added to make the code more readable.
- The `ValueParser$TimespanUnit` class was made a top-level class for unit handling.
- A `Utils::multiplyOverflow(long, long, long)` method was added to avoid negative values in case of an overflow.
- `String::strip` was used instead of `String::trim` to make the implementation Unicode aware.
- The cutoff setting in the `EventControl` class was changed to “infinity”, which in practice was already the default value.

Composition, which is typically preferable over inheritance, was previously used for the `EnabledSetting` and `StackTraceSetting` classes. However, the code became hard to read and error-prone, so there is now an abstract `BooleanSetting` class to enforce consistency.

**Testing:**
 - test/jdk/jdk/jfr/*
 - 100 * test/jdk/jdk/jfr/api/setting/TestSettingControl.java 

**Out of scope:**
- Formatting issues
- Wider use of the TimespanUnit enum
- Clarify the SettingControl specification on how invalid values should be handled

-------------

Commit messages:
 - Minor update
 - Initial

Changes: https://git.openjdk.org/jdk/pull/20336/files
  Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=20336&range=00
  Issue: https://bugs.openjdk.org/browse/JDK-8335907
  Stats: 911 lines in 17 files changed: 573 ins; 264 del; 74 mod
  Patch: https://git.openjdk.org/jdk/pull/20336.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/20336/head:pull/20336

PR: https://git.openjdk.org/jdk/pull/20336


More information about the hotspot-jfr-dev mailing list