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