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