RFR: 8249: Add support for the new allocation profiler in rules
Carter Kozak
duke at openjdk.org
Fri Aug 30 19:19:53 UTC 2024
On Thu, 22 Aug 2024 20:50:58 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 :-)
core/org.openjdk.jmc.flightrecorder/src/main/java/org/openjdk/jmc/flightrecorder/jdk/JdkAttributes.java line 583:
> 581: return JdkAttributes.ALLOCATION_SIZE.getAccessor(type);
> 582: } else if (type.getIdentifier().equals(JdkTypeIDs.OBJ_ALLOC_SAMPLE)) {
> 583: return JdkAttributes.SAMPLE_WEIGHT.getAccessor(type);
I'm going to revert this piece and branch on whether we expect ObjectAllocationSample events -- I initially audited uses within `core`, however I missed some places in the UI that would likely have slightly different behavior.
I suspect it would be generally helpful to include sample weight in this attribute, but if we make that change, I'd prefer to do it in isolation from this enhancement.
core/tests/org.openjdk.jmc.flightrecorder.rules.jdk.test/src/main/java/org/openjdk/jmc/flightrecorder/test/rules/jdk/TestRulesWithJfr.java line 270:
> 268: StreamResult console = new StreamResult(os);
> 269: transformer.transform(source, console);
> 270: } catch (TransformerException e) {
I added this test to the `TestRulesWithJfr.java` class to reuse `generateReport` without a large diff -- if we move forward with this approach, it may make sense to share the rule report utility functionality
-------------
PR Review Comment: https://git.openjdk.org/jmc/pull/579#discussion_r1739294264
PR Review Comment: https://git.openjdk.org/jmc/pull/579#discussion_r1727799033
More information about the jmc-dev
mailing list