-
Notifications
You must be signed in to change notification settings - Fork 164
Make UnifiedGCLogParser public #464
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Make UnifiedGCLogParser public #464
Conversation
Make it possible to extend it in third party packages.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR makes the UnifiedGCLogParser class public to enable third-party packages to extend it, addressing a use case where custom parsers need to extend this base class to parse additional JVM startup information. The PR author currently works around this limitation by extending UnifiedGenerationalParser instead.
Changes:
- Changed
UnifiedGCLogParserclass visibility from package-private to public - Changed
advanceClock(String record)method visibility from package-private to protected to support overriding in third-party extensions
Comments suppressed due to low confidence (2)
parser/src/main/java/com/microsoft/gctoolkit/parser/UnifiedGCLogParser.java:18
- The advanceClock method is now protected, enabling third-party extensions. However, two other package-private methods in this class (notYetImplemented and noop) are also used by existing subclasses like UnifiedG1GCParser and UnifiedGenerationalParser. For complete extensibility, these methods should also be changed to protected visibility so third-party implementations can use them.
protected void advanceClock(String record) {
parser/src/main/java/com/microsoft/gctoolkit/parser/UnifiedGCLogParser.java:18
- The advanceClock method is now protected and part of the public API surface for third-party extensions. However, it lacks JavaDoc documentation. Consider adding JavaDoc that describes its purpose, parameter, and the exception handling behavior (catches all Throwables and logs them at FINE level).
protected void advanceClock(String record) {
parser/src/main/java/com/microsoft/gctoolkit/parser/UnifiedGCLogParser.java
Show resolved
Hide resolved
parser/src/main/java/com/microsoft/gctoolkit/parser/UnifiedGCLogParser.java
Show resolved
Hide resolved
…ogParser.java Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Copilot makes good points Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
@kcpeppe - CCing you in in case you still wanted to comment on this but I think enabling 3P extensibility is an OK goal for this. |
Make it possible to extend it in third party packages.
We have an implementation of this, and it currently extends from
UnifiedGenerationalParserfor no other reason that it is public.For more information, there is nothing very exciting about our custom implementation, it just collects all the info printed on start: