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

LogCollector Incorrectly Aggregates Top-level Frames #179

Open
PhRX opened this issue Jan 18, 2017 · 3 comments
Open

LogCollector Incorrectly Aggregates Top-level Frames #179

PhRX opened this issue Jan 18, 2017 · 3 comments

Comments

@PhRX
Copy link
Contributor

PhRX commented Jan 18, 2017

The LogCollector works based on the assumption that for any given thread, there's only 1 top-level method. That however isn't strictly true, looking at some real logs. The top of a stack typically is something like java.lang.Thread.run(), but in the main thread, the call can be preceded in time by classloading operations. For instance : sun.launcher.LauncherHelper.checkAndLoadMain(), at the same level as the Thread.run() call.

Because the aggregation assumes only 1 top-level frame, later stackframes may all end up in the wrong ProfileTree. In the example above, say that main calles com.a.b(), you might end up with an aggregated situation which looks like the sun.launcher.[...] is calling com.a.b().

This is fixed in my latest commit (the aggregation has been reworked), but that's on a dev branch which isn't yet integrated into the integration branch on my fork (and even further away from a Pull request).

@nitsanw
Copy link
Member

nitsanw commented Jan 19, 2017

Can you isolate the fix into a PR, or point to the fixing commit on your fork?

@PhRX
Copy link
Contributor Author

PhRX commented Jan 19, 2017

@nitsanw Actually, that's not quite possible.
I've reworked the aggregation, which is how I discovered it (by comparing the differences between what I'm seeing in my current build with the "original" HP UI). So the "fix" is inside the new classes, and can't really be isolated.
Hopefully I can finish my current branch today (I need to add the filtering logic in the new structure), and integrate into the integration branch on my fork. The head there will then contain something which is functionally equivalent with what is at the head of the current Pull Request, but working on a new aggregation paradigm which is a bit more generic, and with the log collection decoupled more from the UI.

@PhRX PhRX mentioned this issue Feb 7, 2017
@PhRX
Copy link
Contributor Author

PhRX commented Mar 9, 2017

Thinking of closing this but I didn't check the console. The issue is in LogCollector, so I suspect this issue should remain open until console is fixed too.

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

No branches or pull requests

2 participants