RFR: 8253641: Missing newline in the printout of certain JFR events [v2]

[Markus Grönlund]

On Tue, 20 Oct 2020 13:43:35 GMT, Michael Bien <github.com+114367+mbien at openjdk.org> wrote:

>> Trivial fix which adds a missing new line to PrettyWriter::printClassLoader if clazz == null.
>> 
>> PrettyWriter is used for both the jfr CLI tool and also for the impl of the public JFR API behind
>> jdk.jfr.Event::toString().
>> how to reproduce using JFR CLI:
>> 
>> To generate a JFR dump for testing, simply start HelloWorld.java
>> public class HelloWorld {
>>     public static void main(String[] args) {System.out.println("hello there");}
>> }
>> with
>> `jdk-15+36/bin/java -XX:StartFlightRecording=filename=dump.jfr,dumponexit=true HelloWorld.java`
>> 
>> 
>> current result for events where the classLoader field equals null (note: newline missing):
>> 
>> jdk-15+36/bin/jfr print --events jdk.ModuleRequire dump.jfr
>> ...
>> jdk.ModuleRequire {
>>   startTime = 16:12:43.524
>>   source = {
>>     name = "jdk.nio.mapmode"
>>     version = "15"
>>     location = "jrt:/jdk.nio.mapmode"
>>     classLoader = null  }
>>   requiredModule = {
>>     name = "java.base"
>>     version = "15"
>>     location = "jrt:/java.base"
>>     classLoader = null  }
>> }
>> ...
>> 
>> after the patch, using same jfr dump and displaying the same event:
>> 
>> jdk16/build/linux-x86_64-server-release/jdk/bin/jfr print --events jdk.ModuleRequire dump.jfr
>> ...
>> jdk.ModuleRequire {
>>   startTime = 16:12:43.524
>>   source = {
>>     name = "jdk.nio.mapmode"
>>     version = "15"
>>     location = "jrt:/jdk.nio.mapmode"
>>     classLoader = null
>>   }
>>   requiredModule = {
>>     name = "java.base"
>>     version = "15"
>>     location = "jrt:/java.base"
>>     classLoader = null
>>   }
>> }
>> ...
>> 
>> sidenote:
>> 
>> There would be an opportunity to merge the two sequential if-else blocks[1] in printClassLoader over the clazz variable
>> after this patch, but this would obfuscate the actual bug fix, so I decided to omit it. Let me know if I should add
>> that. [1]
>> https://github.com/mbien/jdk/blob/67750e2b599b3f2a6b814d7bceada785ca4051f9/src/jdk.jfr/share/classes/jdk/jfr/internal/tool/PrettyWriter.java#L440-L448
>
> Michael Bien has refreshed the contents of this pull request, and previous commits have been removed. The incremental
> views will show differences compared to the previous content of the PR. The pull request contains two new commits since
> the last revision:
>  - small refactoring: merged two if blocks in PrettyWriter::printClassLoader.
>  - bugfix: Missing newline in PrettyWriter::printClassLoader if classloader is null.

src/jdk.jfr/share/classes/jdk/jfr/internal/tool/PrettyWriter.java line 446:

> 444:             print(String.valueOf(cl.getId()));
> 445:             print(")");
> 446:         }else{

please add spaces: } else {

src/jdk.jfr/share/classes/jdk/jfr/internal/tool/PrettyWriter.java line 449:

> 447:             print("null");
> 448:         }
> 449:         println(postFix);

Categorical postFix now possible by changing 445 from println(")"); to print(")");?

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

PR: https://git.openjdk.java.net/jdk/pull/248