-
Notifications
You must be signed in to change notification settings - Fork 156
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
GCToolKit#addDataSourceParser #288
Conversation
this.completionTask = null; | ||
if (t != null) | ||
Executors.newSingleThreadExecutor().execute(t); | ||
if (completionTask != null) |
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.
The reason for this change is that it is possible for an Aggregator to get events from more than one EventSource. If the completionTask is nulled out, then the Phaser only gets decremented by the first EventSource but not the subsequent EventSources.
The test CaptureJVMTerminationEventTest was hanging because of this issue.
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.
I think DRY is being broken a little with the debugging implementation
...er/src/test/java/com/microsoft/gctoolkit/parser/ConcurrentMarkSweepPhaseParserRulesTest.java
Outdated
Show resolved
Hide resolved
api/src/main/java/com/microsoft/gctoolkit/message/DataSourceParser.java
Outdated
Show resolved
Hide resolved
...er/src/test/java/com/microsoft/gctoolkit/parser/ConcurrentMarkSweepPhaseParserRulesTest.java
Outdated
Show resolved
Hide resolved
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.
LGTM....
Using loadDataSourceParser causes the code to skip loading DataSourceParsers using the SPI. We added addDataSourceParser which adds a DataSourceParser to the ones that are loaded with the SPI. Along the way, we noticed that the Aggregators are loaded in such a way that they might never actually be used, which would lead to a hang because the Phaser in AbstractVirtualMachine might never be decremented for those Aggregators that are never used. So, a good deal of this change is to fix that issue, which we did by having the DataSourceParser report what events it produces (added DataSourceParser#producesEvents()). Now the code can check if any DataSourceParsers produce the events that an Aggregator aggregates. If not, the Aggregator is not used.
So... The files to focus on are GCToolkit and AbstractJavaVirtualMachine