RFR: 8317562: [JFR] Compilation queue statistics

Martijn Verburg karianna at openjdk.org
Wed Oct 18 15:28:19 UTC 2023


On Tue, 17 Oct 2023 00:35:54 GMT, Mat Carter <macarte at openjdk.org> wrote:

> Adding a new periodic jfr event to monitor and output statistics for the compiler queues.  You will see one event per compiler queue (c1 and c2)
> 
> Passes tier1 on linux (x86) and mac (aarch64)

very nitpicky - take or leave the suggestions :-)

src/hotspot/share/compiler/compileBroker.hpp line 121:

> 119:   int          size()     const                  { return _size;          }
> 120: 
> 121:   int         get_peak_size()     const          { return _peak_size; }

whitespace looks out by one?

src/hotspot/share/jfr/metadata/metadata.xml line 861:

> 859:     <Field type="long" contentType="count" name="totalAdded" label="Total Added" description="Total requests"/>
> 860:     <Field type="long" contentType="count" name="totalRemoved" label="Total Removed" description="Total removed requests"/>
> 861:     <Field type="int" name="compilerThreadCount" label="Number of Compiler Threads for this queue" />

Suggestion:

    <Field type="int" name="compilerThreadCount" label="Number of Compiler Threads for this queue"/>

src/hotspot/share/jfr/periodic/jfrCompilerQueueUtilization.cpp line 37:

> 35: };
> 36: 
> 37: // If current counters are less than previous we assume the interface has been reset

Suggestion:

// If current counters are less than previous, we assume the interface has been reset

src/hotspot/share/jfr/periodic/jfrCompilerQueueUtilization.cpp line 49:

> 47: void JfrCompilerQueueUtilization::send_events() {
> 48:   static CompilerQueueEntry compilerQueueEntries[2] = {
> 49:     {CompileBroker::c1_compile_queue(), 1, 0,0},

Suggestion:

    {CompileBroker::c1_compile_queue(), 1, 0, 0},

src/hotspot/share/jfr/periodic/jfrCompilerQueueUtilization.cpp line 50:

> 48:   static CompilerQueueEntry compilerQueueEntries[2] = {
> 49:     {CompileBroker::c1_compile_queue(), 1, 0,0},
> 50:     {CompileBroker::c2_compile_queue(), 2, 0,0}};

Suggestion:

    {CompileBroker::c2_compile_queue(), 2, 0, 0}};

src/hotspot/share/jfr/periodic/jfrCompilerQueueUtilization.cpp line 55:

> 53:   static JfrTicks last_sample_instant;
> 54:   const JfrTickspan interval = cur_time - last_sample_instant;
> 55:   for(int i = 0; i < 2; i ++)

2 is a magic number, maybe a comment above the for declartion explaining why?

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

Changes requested by karianna (no project role).

PR Review: https://git.openjdk.org/jdk/pull/16211#pullrequestreview-1681156472
PR Review Comment: https://git.openjdk.org/jdk/pull/16211#discussion_r1361399313
PR Review Comment: https://git.openjdk.org/jdk/pull/16211#discussion_r1361399583
PR Review Comment: https://git.openjdk.org/jdk/pull/16211#discussion_r1361399733
PR Review Comment: https://git.openjdk.org/jdk/pull/16211#discussion_r1361400079
PR Review Comment: https://git.openjdk.org/jdk/pull/16211#discussion_r1361400114
PR Review Comment: https://git.openjdk.org/jdk/pull/16211#discussion_r1361400356


More information about the hotspot-jfr-dev mailing list