Closed farajidaneshgar closed 5 days ago
Can you make the build pass?
I tried using it yesterday.
I have a use case that can use it. I can share the traces, but I cannot even get it to work at the moment!
Hello Matthew, Yes. I have new push eliminating that dependencies and adding some other features. I will pull it today.
On Wed, Apr 24, 2024 at 9:23 AM Matthew Khouzam @.***> wrote:
I tried using it yesterday.
- The code doesn't build on HEAD. This is because Arnaud deprecated the incubator callstack
- There is open tracing changes in the first patch, they should be a separate patch
- splitting the code into "first patch" "second patch" and "third patch" is not descriptive enough. We need to have titles that describe what they do.
I have a use case that can use it. I can share the traces, but I cannot even get it to work at the moment!
— Reply to this email directly, view it on GitHub https://github.com/eclipse-tracecompass-incubator/org.eclipse.tracecompass.incubator/pull/13#issuecomment-2074940704, or unsubscribe https://github.com/notifications/unsubscribe-auth/AJIHYVI6Q6AFGNYQ5TIZQGTY66W35AVCNFSM6AAAAABE74PPIWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDANZUHE2DANZQGQ . You are receiving this because you authored the thread.Message ID: <eclipse-tracecompass-incubator/org.eclipse.tracecompass. @.***>
Just writing this comment to update the situation:
please update commit messages, also thanks for fixing the pr!
Hello! Are there screenshots or a video screen capture that could be used to give feedback on the result without needing to do the whole development setup? My understanding is that building it myself is currently the only way.
Hello! Are there screenshots or a video screen capture that could be used to give feedback on the result without needing to do the whole development setup? My understanding is that building it myself is currently the only way.
Reminder that it would be great to have some screenshots or a video capture showing the feature. It would allow folks who do not have the Trace Compass dev setup to review.
Hi ebugden, here is some screenshots:
Thank you for the screenshots and for your work on this analysis! :sunflower: It's practical to be able to see the result visually in order to give feedback.
Some elements I like:
I have some questions and comments about the visualisation.
I noticed that there is a label of "GroupB-GroupA" next to the chart, but it's still not clear what data the chart contains. Some questions that came to mind were:
At the moment it's unclear to me what kind of things it is appropriate to compare with this visualisation. I feel that answering the above questions (in the "Clarify how to read the chart" section) would improve this.
I included screenshots annotated with my comments below.
Some important themes:
Adding comments from an offline discussion.
This was merged! :) :) :) :+1: :100:
execution comparison: -Introduce a module to compare two groups of executions using a differential flame graph. -An "EventDensityView" is used to select a time range for each group and Select the desired traces in an experiment. -Showing the difference ratio based on self time required a modification to AggregatedCalledFunction. -Introducing "ParametricWeightedTreeUtils" to get "SelfTime" instead of "duration" while building the differential Trees. -Adding the option of textual input of dates and query to build the differential flame graph. -moving grouping to hamburger menu -Differential flame graph view is updated to be able to show and hide Query part by demand.
remove commented code: Done improve commit messages, no "second commit" and "third commit":Done fix dependencies:Done update aggregatecallsite to have functions like getWeight(string). This doesn't compile locally:Done
Dependencies to the incubator callstack still exist and need to be removed: Done Modifications to the incubator callstack need to be moved to trace compass core profiling plugin.:Done