Skip to content
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

feat: gc log analyzer supports metric #237

Merged
merged 2 commits into from
Nov 2, 2023
Merged

Conversation

weixlu
Copy link
Contributor

@weixlu weixlu commented Oct 16, 2023

No description provided.

@D-D-H
Copy link
Contributor

D-D-H commented Oct 20, 2023

Please sign the eca.

@D-D-H D-D-H requested a review from leveretconey November 1, 2023 07:15
@NoArgsConstructor
@Slf4j
public class GCLogAnalyzer {
private final Map<Map<String, String>, GCLogParser> parserMap = new HashMap<>();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we need to use ConcurrentHashMap if there could be multiple instances analyzing simultaneously.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

Pattern.compile("[\\d-T:+.]+ (\\d+\\.\\d+): \\[[\\s\\S]*"),
Pattern.compile("\\[(\\d+\\.\\d+)s][\\s\\S]*"),
Pattern.compile("(\\d+\\.\\d+): \\[[\\s\\S]*")
};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

timestampPatternToChoose can be declared as a static member of the Context class so that the regular expressions will not be recompiled every time the function is called.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

[2023-08-25T14:28:44.980+0800][0.076s] GC(374) Pause Mark Start 4.459ms
2022-11-28T14:57:05.341+0800: 6.340: [CMS-concurrent-mark-start]
[7.006s] GC(374) Pause Mark Start 4.459ms
675.461: [CMS-concurrent-mark-start]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there are more common patterns like
[2023-08-25T14:28:44.980+0800] GC(374) Pause Mark Start 4.459ms
2022-11-28T14:57:05.341+0800: [CMS-concurrent-mark-start]
are these regular expressions enough to cover these cases?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, more patterns can be found in gclog, which makes things complex. Luckily, as agreed with our users, both uptime and datetime should be recorded in gclogs. Then we are able to focus barely on this case.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, more patterns can be found in gclog, which makes things complex. Luckily, as agreed with our users, both uptime and datetime should be recorded in gclogs. Then we are able to focus barely on this case.

class ContextComparator implements Comparator<String> {

boolean isPrecedent(String str) {
Set<String> precedentPatternSet = new ImmutableSet.Builder<String>().add("Pause Young", "Pause Initial Mark", "Concurrent Cycle", "Concurrent Mark").build();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can also be declared as static member to reduce object creation.
And I think this list may not be sufficient. For example, "Pause Initial Mark" only appears in JDK 11's CMS, while in JDK 8 it is "CMS Initial Mark".

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm afraid this comparator can't be static, because it relies on the specific timestamp pattern of current gc log. This won't impact performance much, though, because class Context is created once for each gc logs.

Map<String, String> sharedLabels = new HashMap<>(instanceId);
sharedLabels.put("gc_type", gcModel.getCollectorType().getName());
for (GCEvent gcEvent : gcModel.getGcEvents()) {
long timestamp = (long)(gcModel.getReferenceTimestamp() + gcEvent.getStartTime());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Based on current design, getReferenceTimestamp may be UNKOWN (i.e -1) if datesteamp is not printed in the log. I think this situation may require some additional handling.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As explained before, this feature requires datestamp to be printed. If not, an exception will be raised at the very beginning(i.e. before reaching here)

.put("Young", MemoryArea.YOUNG)
.put("Old", MemoryArea.OLD)
.put("Heap", MemoryArea.HEAP)
.put("Metaspace", MemoryArea.METASPACE)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And there is Humongous for g1

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I will add it.

e.printStackTrace();
}
double spendTime = (System.currentTimeMillis() - beginTime) / 1000.;
System.out.printf("分析%s,共计%d条数据,生成%d条metrics,耗时%fs,平均1分钟可处理%f条数据,生成%f条metrics\n",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use English, like all other code and comments

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

@D-D-H D-D-H merged commit 3677aca into eclipse-jifa:main Nov 2, 2023
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants