Integrated: 8335907: JFR: Make SettingControls more robust
Erik Gahlin
egahlin at openjdk.org
Tue Jul 30 13:50:45 UTC 2024
On Thu, 25 Jul 2024 19:59:11 GMT, Erik Gahlin <egahlin at openjdk.org> wrote:
> Could I have a 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
This pull request has now been integrated.
Changeset: 41486131
Author: Erik Gahlin <egahlin at openjdk.org>
URL: https://git.openjdk.org/jdk/commit/41486131481164a559aa534807fe1a77a7d29fc8
Stats: 907 lines in 17 files changed: 569 ins; 264 del; 74 mod
8335907: JFR: Make SettingControls more robust
Reviewed-by: mgronlun
-------------
PR: https://git.openjdk.org/jdk/pull/20336
More information about the hotspot-jfr-dev
mailing list