Support to parse PrintGCID

[xiangzhai]

Hi,

Fixed #197

Testcase:

Command Line:

-Xloggc:specjbb2015-psgc-`date +%Y%m%d%H%M%S`.log -XX:+PrintGCDetails -XX:+PrintGCDateStamps -XX:+PrintGCTimeStamps -XX:+PrintGCID

GC log:

2022-07-26T23:30:28.892+0800: 8991.259: #353: [Full GC (System.gc()) [PSYoungGen: 384K->0K(7465472K)] [ParOldGen: 325752K->325474K(1048576K)] 326136K->325474K(8514048K), [Metaspace: 20971K->20971K(1069056K)], 0.4225592 secs] [Times: user=1.49 sys=0.01, real=0.43 secs]

Please review my patch.

Thanks,
Leslie Zhai

[codecov-commenter]

Codecov Report

Merging #260 (e5771d5) into develop (fd90b9d) will decrease coverage by 0.16%.
The diff coverage is 20.83%.

@@              Coverage Diff              @@
##             develop     #260      +/-   ##
=============================================
- Coverage      70.40%   70.24%   -0.17%     
- Complexity      1528     1529       +1     
=============================================
  Files            146      146              
  Lines           8660     8683      +23     
  Branches        1399     1406       +7     
=============================================
+ Hits            6097     6099       +2     
- Misses          1973     1993      +20     
- Partials         590      591       +1     

Impacted Files	Coverage Δ
...traum/perf/gcviewer/imp/AbstractDataReaderSun.java	78.13% <5.55%> (-5.01%)	⬇️
...gtraum/perf/gcviewer/imp/DataReaderSun1_6_0G1.java	89.06% <50.00%> (-0.55%)	⬇️
...tagtraum/perf/gcviewer/imp/DataReaderSun1_6_0.java	90.06% <100.00%> (+0.03%)	⬆️
.../tagtraum/perf/gcviewer/model/AbstractGCEvent.java	86.12% <100.00%> (+0.03%)	⬆️
...gtraum/perf/gcviewer/ctrl/impl/GcSeriesLoader.java	74.15% <0.00%> (-2.25%)	⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us.

[YouriAndropov]

Hello, good news !
I have no env for building it, where could I find a jar file ?

[xiangzhai]

Hi @YouriAndropov

Please try it gcviewer-1.37-SNAPSHOT.jar.tar.gz

Thanks,
Leslie Zhai

[chewiebug]

The code looks good at first glance - thank you for your implementation! I miss some unittests, though. Best regards Jörg

[xiangzhai]

Hi Jörg,

Thanks for your response!

I miss some unittests, though.

Regression? Please point out the testcase.

Thanks,
Leslie Zhai

[chewiebug]

Hi Leslie You didn't introduce a regression, the tests done during the pull request build would have found them. The build result is still red, because code coverage percentage drops with this pull request. I miss unittests proving that your code works. They will also make sure that no later code changes will break your implementation and they'd prevent the drop in code coverage for your pull request. Doing some manual tests myself, I suspect, that you mainly tested the parallel collector because that's the only one parsing without warnings. Serial, cms and g1 have lots of parser warnings. It is ok to add a new feature for one garbage collection algorithm only. I usually add a comment the implementation to make that fact clear (class comment of DataReaderSun1_6_0.java in this case). So I see the following options for this pull request: * add some unittests for parallel collector (as part of the class TestDataReaderSun1_8_0.java) and a comment in DataReaderSun1_6_0.java stating the restriction PringGCID only for parallel collector) * add unittests for more collectors (TestDataReaderSun1_8_0.java and TestDataReaderSun1_8_0G1.java, if G1 should also be included) What would you like to do? Best regards Jörg

[chewiebug]

Please also add some unittests proving that your code works as expected and preventing future changes from breaking your implementation.

[xiangzhai]

Hi,

Thanks for your review!

Doing some manual tests myself, I suspect, that you mainly tested the
parallel collector because that's the only one parsing without warnings.
Serial, cms and g1 have lots of parser warnings.

Yup, we mainly use Parallel Scavenge collector for jdk8u-dev, G1 GC for jdk11u-dev, G1GC and ZGC for jdk17u-dev.

add some unittests for parallel collector (as part of the class
TestDataReaderSun1_8_0.java) and a comment in
DataReaderSun1_6_0.java stating the restriction PringGCID only for
parallel collector)

Yup, I added some unittests only for Parallel Scavenge collector and comment the restriction PringGCID only test for it.

Please review it again.

Thanks,
Leslie Zhai

[xiangzhai]

PING @chewiebug

[xiangzhai]

PING

[xiangzhai]

PING

[xiangzhai]

PING

[chewiebug]

Hi Leslie The pull request looks good now, thank you very much! Did both you and Weilung Cui contribute to this pull request? I am asking because I'd like to add everyone, who contributed to the list of contributers. Best regards Jörg

[CuiWeiloong]

Hi Jörg,

Thanks for the mention. I didn't contribute to this pull request, but I recently fixed some failed parsing for ZGC and ShenandoahGC and will create a PR soon.

Best regards
Weilong Cui

[xiangzhai]

Hi Jörg,

Did both you and Weilung Cui contribute to this pull request?

I contributed to this PR.

Thanks,
Leslie Zhai

[xiangzhai]

PING @chewiebug

[chewiebug]

I have merged the pull request. Unfortunately, there seems to be an issue with the file upload to sf.net, so the latest build is not uploaded, yet. I am investigating the issue. Please be patient... Best regards Jörg

[chewiebug]

build should be fixed (is currently running)