-
-
Notifications
You must be signed in to change notification settings - Fork 706
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
[multicapture 3/3] Add multi-capture capabilities to capture
+ merge utility
#825
base: master
Are you sure you want to change the base?
Conversation
It may be worth considering if this can be done within the
It's ok. |
I can have a look into that |
Some personal notes after a short glance: ➕ we can definitely do better in
➖ but I fear we won't be able to seamlessly integrate multi-capture
Next steps;
I'll push that further to the code and see how it goes |
I have no idea, but there are programs that are capable of doing that. Maybe the readline library would be of use here?
Valid points, but not show-stoppers. It may make sense to split the available options into three categories:
|
For me that was just about editing a single line (if you indeed mean https://tiswww.case.edu/php/chet/readline/rltop.html) Since then I've found this SO thread - I'll have to test it but that might do the trick for linux at least : https://stackoverflow.com/a/11474509/7002298 |
I updated the branch with an updated General refactoring
Integration of multi-capture in
|
Quite happy about this live update, what do you think ? tracy-multi-capture-liveupdate.webm |
It seems to be a bit busy due to what I assume is debug info. Can you show a version that just adds a new line when another connection is established? |
8cc2909
to
2b8af30
Compare
tracy-multi-capture-liveupdate-clean.webm |
Looks great! |
Marking the MR as ready for review
Regarding merge strategy, I can spin up smaller MRs with more atomic changes if you want to review things in multiple steps. It would look like
|
It seems there are some commits here which are intended as fixups in a rebase. |
da877bd
to
26df552
Compare
not sure what you mean by that; in any case I rebased and cleaned up the branch history to clarify the separation A/B/C as mentioned in #825 (comment)
|
I extracted the non-functional refactors into #837 to simplify the review, and make sure those changes are not impacting anything. That may be a good starting point to merge the changes, and will reduce the diff here. Same for #838 for the merge utility that has no interaction with the changes in |
capture
+ merge utility
Note: if you're interested, I can also spin up a dedicated PR for the internal refactor in |
Please run the changes you are proposing through clang-format. At this point it's hard to review. |
26df552
to
0848bb1
Compare
Sure ! Just did it on this MR. To clarify, do you intend to review separately #837 and #838, in which case I'll continue to keep them up-to-date, or will you tackle the whole change as a whole ? I'm was trying to save you time by splitting the work this way, but I'm not sure it's actually helping you - please clarify what you prefer :) |
Yes, please do the other PRs too. |
2511616
to
aa451b4
Compare
Hi ( somewhat new to github dont hesitate to direct me to the correct place for this if it's not here )
are not visible after a merge This code when captured and launched on the profiler gives this trace
however after running it through the tracy-merge util the lock as well as CPU usage and ZoneText all disappear |
Hi @afaure42 ! first of all, thanks for taking the time to test this branch :) Sadly, the limitations you mentioned are expected - cf the first message of the MR -> 2nd limitation
I'm not saying it's impossible to do, but this would require more work. IIRC (I did this MR in summer), including those infos would require some reworks in the export/import process I'm using for this feature (we cannot yet import those infos via the existing |
Thanks for your answer @Arpafaucon ! I understand the limitations, i'll look into it maybe i can give it a try ! |
suggestion towards #471
Description
While a network-level multiplex seems the most transparent way to implement multiprocess support, @cipharius and I have seen that it requires an advanced and very precise knowledge of the packets in transit. Thus there is an inherent fragility to it (cf #766 (comment)).
Here, I wanted to propose a "dumber" and more stable approach that does not have to know a lot about the internals. My goal by doing so was to provide a first step to support multi-process workflows, with a minimal amount of maintenance.
In the limited amount of work time I got, this looked like a more reasonable approach to explore first.
The general approach
multi-capture
is a "generalized" version oftracy-capture
that listens to UDP discovery packets. Its job is to run one "capture" per discovered process. In more details, this boils down to creating multipleWorker
instances pointing to the right addresses, and let them do their job.merge
can combine multiple trace files into a single one. Its strategy isWorker
to read it (like we do when opening the profiler GUI)ImportEventTimeline
,ImportEventMessages
...import-chrome
/import-fuchsia
Pros
Worker
internals, nor do assumptions about the way things are stored / exchanged on the networkmulticapture
is useful too: it helps dealing with the fact that multiple tracy-enabled processes might be living at the same time, and the user might not know which one the standardcapture
will talk too.Cons:
While (1) is definitely a consequence of the design, I think (4) is totally fixable. We can work gradually work to improve the state of (2) and (3) by iterating on the existing import interface (that could benefit the
import-chrome/fuchsia
utilities too) and on the export process. It might be even possible to directly merge multipleWorker
s, but I haven't looked into that.I am very aware that this is NOT the design path @wolfpld suggested when discussing multi-process support; so I wanted to show you the code as soon as I felt it worthy of your time.
As mentioned in the pros, it might well also be that
multicapture
is a sufficient solution on its own for some use cases. For my individual experience, this would already solve most of the pain when profiling a library used by several programs running together - but I can't presume whether this is a frequent pain point.How to reproduce / test
Capture time
tracy-multicapture
with a prefix nametracy-multi-capture-reenc.webm
Merge
tracy-merge
Visualize