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

Overlapping text on adjacent spans that also overlap #15

Closed
danatcofo opened this issue Mar 24, 2022 · 18 comments
Closed

Overlapping text on adjacent spans that also overlap #15

danatcofo opened this issue Mar 24, 2022 · 18 comments

Comments

@danatcofo
Copy link

When 2 adjacent spans have starts and durations that cause an overlap, the first spans text is not occluded by the second span. See the top highlighted call out in the image above.

image

NOTE: This is a holder issue for me as I'm going to try and fix it myself.

@pyatyispyatil
Copy link
Owner

It seems to be preferable to use the waterfall plugin for this situation - move your requests data to the waterfall section on initialization and all will be fine.)

The flame chart (as a concept, not this project) does not allow situations where you can have two nodes overlap in duration.

@danatcofo
Copy link
Author

It's still the best visualization for the idea of a trace however. the difference being the concept of async function calls. real world data HAS async function calls so ensuring this can support those visualizations cleanly is ideal. Even modern javascript has the concept of async calls now.

@Smrtnyk
Copy link
Collaborator

Smrtnyk commented Mar 24, 2022

those async functions are awaited time, maybe that time could be colored with some diagonal lines to represent them as async

@pyatyispyatil
Copy link
Owner

In the javascript world, all asynchronous operations are still executed sequentially (because we have the event loop) and there can be no overlap. The exception occurs only if there are multiple threads. And in this case, you need to build several flame charts and this can be done using the plugin system.

Specifically, requests are displayed separately from code execution.
image

@Smrtnyk
Copy link
Collaborator

Smrtnyk commented Mar 24, 2022

Capture
This is how we visualised our async functions.
The part with diagonal lines was a passive (async time) and other part with flat color is an active function time.

@danatcofo
Copy link
Author

so... while we have all been discussing this.. this DOES allow you to have overlapping time frames... I'm doing some final testing but I believe I've got all the cases figured out to support it smoothly. My coming PR also simplifies the render to a single render queue that is called sequentially rather than having 3 render queues that are called sequentially. It was the 3 render queues that caused the issue here as the text renders occurred after ALL the rectangle renders. This mean for the canvas. it placed text over everything even though the calls to add to queue happened in a different order. By collapsing the queues to a single one we can render in order with the expectation that last in wins.

Additionally adding in a simple fix to properly grab the hit-region for overlapping spans. It's pretty slick and the changes look to be just as performant.

PR incoming soon.

danatcofo pushed a commit to stackify/flame-chart-js that referenced this issue Mar 24, 2022
…ct hit boxing and text to also overlap

* Updated render queues to be a single render queue
* Updated hit box searching to find the last hit rather than the first hit under assumption that last hit is the region 'on top'
* Fixed waterfall text render to occur after rectangle render
@Smrtnyk
Copy link
Collaborator

Smrtnyk commented Mar 24, 2022

Could you provide a code snippet that would cause such a trace with overlapping frames?

@danatcofo
Copy link
Author

danatcofo commented Mar 24, 2022

I updated the demo to randomly generate such an example. i.e. during generation of frames it will randomly create a frame that overlaps.

@Smrtnyk
Copy link
Collaborator

Smrtnyk commented Mar 24, 2022

yes. but that is manually generated data.
Can you provide a javascript code snippet that would produce such a trace when instrumented?

@danatcofo
Copy link
Author

danatcofo commented Mar 24, 2022

image

[overlapping.txt](https://github.com/pyatyispyatil/flame-chart-js/files/8344135/overlapping-exampl.txt)

you can import this data into the demo

@Smrtnyk
Copy link
Collaborator

Smrtnyk commented Mar 24, 2022

Ok, I think there is a misunderstanding here, so I will put it this way
Can you provide a chrome profile where function calls would overlap?
A real usecase where such trace with overlapped frames would happen in javascript world.

Or is this not about javascript at all?
Because I don't see how this scenario could happen in javascript, since it is not a multithreaded language.

@danatcofo
Copy link
Author

this is not javascript... these are server traces of things being profiled like a php, ruby, python, dotnet or java application etc... The flame chart visualization works wonderfully for these types of representations and thats what I use if for.

@pyatyispyatil
Copy link
Owner

I think we should find another solution for this case. As I saw in the PR that you opened, the current solution breaks core engine optimizations and looks like it doesn't solve the problems you want to solve. I think it's preferable to use something like logic in waterfall data with intervals and timings in the same node.

Because the case where we have two or more nodes in the same period also breaks the timeframe plugin logic (in the top plugin in the UI you can see broken line on your screenshot) and possibly broken tree cluster logic.

@Smrtnyk
Copy link
Collaborator

Smrtnyk commented Mar 24, 2022

If you check the picture that I posted above, the diagonal lines are what we achieved with intervals.
Function nodes can have an interval entry which contains start and stop of an interval (which represents the time where function was just waiting for some async stuff)

leonid-granulate pushed a commit to Granulate/flame-chart-js that referenced this issue Dec 18, 2023
@fizerkhan
Copy link

Is this fix released?

@pyatyispyatil
Copy link
Owner

@fizerkhan No.(
I'll deal with this problem later when I have time.
I closed this issue because the discussion was not a very correct solution and the situation described in the first message is divorced from reality, because spans cannot intersect with each other - this will break the tree structure.

@fizerkhan
Copy link

@pyatyispyatil Thanks for the quick response.

In Telemetry data, more than one span can have the same start time and different duration due to its async nature.
Do you think the flame chart is not designed for Async's spans (or parallel spans)? I am just curious.

If you have a better solution, Please suggest it. We will work on it and give you the PR request.

@pyatyispyatil
Copy link
Owner

I think you need to combine the use of waterfall chart and flame chart. In waterfall you can display multiple independent spans with intersecting timelines, and in turn in the flame chart you can display their “content”

If your asynchrony involves multi-threaded execution of spans, that is, each span can execute its own code, then most likely you need to use several independent flame charts and display data in them depending on the thread. There is a similar thing in the examples.

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

4 participants