The RequestEngine class has several shortcomings.
Events with periods that are multiples of each other, for example, events with a period of "1 s" and "5 s", are not run at the same time. This results in the periodic task thread sleeping and waking up an unnecessary number of times, but perhaps more problematic, it makes it harder to correlate measurements, or cache data between events.
Insertion/removal of N periodic events require O(N^2) time. Every time a new event is added an array list is copied. To find the event to remove, a list has to be iterated. A workaround for the JVM events have been implemented, but it makes the code harder to analyze from a vulnerability perspective. Insertion and removal can be done in O(1), but it requires special handling of hashCode and equals if ordering of events is to be preserved. IdentityHashMap will not work.
It's also hard to follow how thread safety is ensured. Events for beginChunk and endChunk don't happen in the same thread as events emitted at an interval.
Furthermore, the RequestEngine.java file has a rather large inner class because it didn't seem appropriate to fill up jdk.jfr.internal package with another class related to periodic events. Suggestion is to move all the logic to a separate package, split up the implementation into smaller classes and change the behavior so events are emitted in batches.
Events with periods that are multiples of each other, for example, events with a period of "1 s" and "5 s", are not run at the same time. This results in the periodic task thread sleeping and waking up an unnecessary number of times, but perhaps more problematic, it makes it harder to correlate measurements, or cache data between events.
Insertion/removal of N periodic events require O(N^2) time. Every time a new event is added an array list is copied. To find the event to remove, a list has to be iterated. A workaround for the JVM events have been implemented, but it makes the code harder to analyze from a vulnerability perspective. Insertion and removal can be done in O(1), but it requires special handling of hashCode and equals if ordering of events is to be preserved. IdentityHashMap will not work.
It's also hard to follow how thread safety is ensured. Events for beginChunk and endChunk don't happen in the same thread as events emitted at an interval.
Furthermore, the RequestEngine.java file has a rather large inner class because it didn't seem appropriate to fill up jdk.jfr.internal package with another class related to periodic events. Suggestion is to move all the logic to a separate package, split up the implementation into smaller classes and change the behavior so events are emitted in batches.
- relates to
-
JDK-8311245 JFR: Remove t.printStackTrace() in PeriodicEvents
- Resolved