RFR: 8249: Add support for the new allocation profiler in rules [v2]

Marcus Hirt hirt at openjdk.org
Mon Sep 9 22:42:10 UTC 2024


On Fri, 6 Sep 2024 16:09:48 GMT, Carter Kozak <duke at openjdk.org> wrote:

>> ## Allocation Rules may use ObjectAllocationSample
>> 
>> The allocation profiling rules (`AllocationByClassRule`, `AllocationByThreadRule`, `AutoBoxingRule`) were developed prior to the introduction of the newer and less invasive ObjectAllocationSample event. I've updated these rules to prefer the more precise events, but when those are unavailable, use the new ObjectAllocationSample events instead. This proved a bit tricky because the rules are scored based on number of events sampled, where the new events may not be emitted for every tlab based on a rate limit, so we must take into account the sample weight and estimate the number of samples as best we can.
>> 
>> The `IRule#getRequiredEvents` API doesn't provide a way to declare _either_ dependencies, so I opted to remove the dependencies in favor of a similar check in the `getResult` implementation. 
>> 
>> ## JfrGenerator Utility
>> 
>> The idea is to provide java sources with code that violates rules. We compile and execute the input sources while capturing a JFR, and execute rules against the results to verify they work as expected.
>> 
>> I like having the input sources available in the test rather than only a JFR recording, because it's not clear whether most JDKs will still create the given recording, or with what configuration. However, the trade-off is that we're only testing against the JVM used to run tests, which represents a subset of all supported versions.
>> This type of test must also compile and execute small Java programs, which is more expensive than executing against an existing recording, and may be more likely to flake if we're not careful.
>> 
>> Anyhow, I thought I would share the idea before moving forward, as always, feedback is appreciated :-)
>
> Carter Kozak has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Fix calculateBalanceScore when large integers are provided

Thanks a lot for this PR Carter! Doing this is long overdue. :)

core/org.openjdk.jmc.testlib/src/main/java/org/openjdk/jmc/flightrecorder/test/util/JfrGenerator.java line 108:

> 106: 		 * invokes {@code consumer} with the resulting recording.
> 107: 		 */
> 108: 		public void execute(RecordingConsumer consumer) throws IOException {

Executing another JVM, to dynamically get recordings, seems both a bit risky and error prone to do as part of the testing. We assume that all the events have already been tested as part of the JDK testing, so this testing will be to ensure that we get the right output from the rule, given a certain input recording. 

I would suggest one of two ways of doing this:
Either simply make or generate a little recording, or some recording, that show the behaviour you are looking for. You can generate them, for example, by adding to https://github.com/thegreystone/recgen. Using the writer to generate them will result in recordings with exactly the shape you want, and they will also be minimal. Though, perhaps a bit unrealistic perhaps.

Or, if you very much want to generate them on the fly, you can do what recgen does, and use the jfr writer to generate them from within the tests themselves.

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

Changes requested by hirt (Lead).

PR Review: https://git.openjdk.org/jmc/pull/579#pullrequestreview-2291067019
PR Review Comment: https://git.openjdk.org/jmc/pull/579#discussion_r1751018434


More information about the jmc-dev mailing list