Community
Participate
Working Groups
Build Identifier: Here is the minimal set of feature I am proposing: - Create two actions Group and Ungroup similar to the reverse debugging actions. - The action will be enabled if the DSF debugger supports it. To be defined service that specify if a DSF debugger supports user groping or not. As part of bug 240208 such interface is being proposed (IExecutionContextTranslator TBD). - When the user selects multiple threads or groups it can create new group that will be the parent of the selected threads or groups. - No group support on the back end yet. - Groups can also be grouped in a new group. - The newly created group will be directly under the process node. - The process cannot be grouped. - All newly created threads will be added to the process node. - Group can be empty. - Calling ungroup on a single group will remove the group – all its children will become children of the process. - Make sure expansion happens if thread belongs to a group (it should - the patch for 240208 tests for that). Next steps: - Groups can be a on the back end as well ( somehow defined by policy). - Some kind of persistency so next launch will keep the groups. We need to define an identity of execution contexts between debug sessions. - Execution control on a group will send the commands to its children or not (depend on some kind of debugger policy). - If groups exists on the back end, there can be different policies ( issue the step/run/halt can go to children of the group or the groups if it exists on the backend. Reproducible: Always
(In reply to comment #0) > Build Identifier: > > Here is the minimal set of feature I am proposing: > - Create two actions Group and Ungroup similar to the reverse debugging > actions. I assume you will use 'commands' instead of 'actions' as for reverse. Also, where will you put the UI of these commands? I don't think they should be on a toolbar but should be in the context menu of the debug view. > - The action will be enabled if the DSF debugger supports it. To be defined > service that specify if a DSF debugger supports user groping or not. > As part of bug 240208 such interface is being proposed > (IExecutionContextTranslator TBD). Great. > - When the user selects multiple threads or groups it can create new group that > will be the parent of the selected threads or groups. Can we mix thread and groups together in a new group? I don't think it is essential yet. > - No group support on the back end yet. ok. Let's get the frontend to work before we involve the backend. > - Groups can also be grouped in a new group. nice > - The newly created group will be directly under the process node. > - The process cannot be grouped. Since multi-process is almost ready, I hope we can keep the design generic enough that we will be able to easily include processes later. In fact, I'm hoping for a grouping solution that allows to group anything, potentially restricted to the elements of the group being of the same type. So we could group threads together, or processes together, or groups together, or anything else that is an IDMContext. > - All newly created threads will be added to the process node. You mean, outside of any group? I think that is good. > - Group can be empty. How? Can you select "group" with no selection in the DV? I think the operation should be disabled in that case. > - Calling ungroup on a single group will remove the group – all its children > will become children of the process. Great. > - Make sure expansion happens if thread belongs to a group (it should - the > patch for 240208 tests for that). Great. > Next steps: > - Groups can be a on the back end as well ( somehow defined by policy). This should be available in GDB by next year, so we'll be able to have this support in open-source. > - Some kind of persistency so next launch will keep the groups. > We need to define an identity of execution contexts between debug sessions. Nice. I hadn't thought of such a persistency. It will be nice. > - Execution control on a group will send the commands to its children or not > (depend on some kind of debugger policy). We can handle this as a separate activity in the multicore debug workgroup, as there was interest from others. > - If groups exists on the back end, there can be different policies ( issue > the step/run/halt can go to children of the group or the groups if it exists on > the backend. Good. This should be great!
> > - Create two actions Group and Ungroup similar to the reverse debugging > > actions. > I assume you will use 'commands' instead of 'actions' as for reverse. Also, > where will you put the UI of these commands? I don't think they should be on a > toolbar but should be in the context menu of the debug view. Yes. I'm using a command and will add two debug view context menu "Group" and "Ungroup". > > - The action will be enabled if the DSF debugger supports it. To be defined > > service that specify if a DSF debugger supports user groping or not. > > As part of bug 240208 such interface is being proposed > > (IExecutionContextTranslator TBD). > Great. Actually the context menus will not be even visible if the debugger doesn't support grouping. > > - When the user selects multiple threads or groups it can create new group that > > will be the parent of the selected threads or groups. > Can we mix thread and groups together in a new group? I don't think it is > essential yet. It will help flush more issue with recursive containers. > > - The newly created group will be directly under the process node. > > - The process cannot be grouped. > Since multi-process is almost ready, I hope we can keep the design generic > enough that we will be able to easily include processes later. In fact, I'm > hoping for a grouping solution that allows to group anything, potentially > restricted to the elements of the group being of the same type. So we could > group threads together, or processes together, or groups together, or anything > else that is an IDMContext. OK. Will consider that. When can I see mutiple processes under the same launch? > > - All newly created threads will be added to the process node. > You mean, outside of any group? I think that is good. > > - Group can be empty. > How? Can you select "group" with no selection in the DV? I think the > operation should be disabled in that case. If you group two threads and the threads terminate the group will become empty. We should handle that case.
(In reply to comment #2) > > > - The action will be enabled if the DSF debugger supports it. To be defined > > > service that specify if a DSF debugger supports user groping or not. > > > As part of bug 240208 such interface is being proposed > > > (IExecutionContextTranslator TBD). > > Great. > Actually the context menus will not be even visible if the debugger doesn't > support grouping. Even better. > > > - When the user selects multiple threads or groups it can create new group that > > > will be the parent of the selected threads or groups. > > Can we mix thread and groups together in a new group? I don't think it is > > essential yet. > It will help flush more issue with recursive containers. So, can we also mix different types of elements? Like grouping threads with processes? Sounds complicated... > > > - The newly created group will be directly under the process node. > > > - The process cannot be grouped. > > Since multi-process is almost ready, I hope we can keep the design generic > > enough that we will be able to easily include processes later. In fact, I'm > > hoping for a grouping solution that allows to group anything, potentially > > restricted to the elements of the group being of the same type. So we could > > group threads together, or processes together, or groups together, or anything > > else that is an IDMContext. > OK. Will consider that. When can I see mutiple processes under the same launch? Today :-) Use GDB 7.2, launch a local session (attach or not) and then press the green 'connect' button on the DV toolbar to attach to more processes. You will have to start the processes outside eclipse first. Note that DSF-GDB does support creating new processes in a running session but I haven't added any UI to request that yet. So you can't try it out. > > > - All newly created threads will be added to the process node. > > You mean, outside of any group? I think that is good. > > > - Group can be empty. > > How? Can you select "group" with no selection in the DV? I think the > > operation should be disabled in that case. > If you group two threads and the threads terminate the group will become empty. > We should handle that case. Good point, I hadn't thought of threads that terminate.
> > > > - When the user selects multiple threads or groups it can create new group that > > > > will be the parent of the selected threads or groups. > > > Can we mix thread and groups together in a new group? I don't think it is > > > essential yet. > > It will help flush more issue with recursive containers. > So, can we also mix different types of elements? Like grouping threads with > processes? Sounds complicated... I think we should make the implementation generic if it helps us make the code cleaner. Also, from a feature side I think makes sense to have less restrictions. Here is a made up example: A server has allocated three of its threads to talk to few other processes in my system. Why shouldn't the user be able to group these three thread and three processes together. Won’t that reduce the clutter in the debug view? Basically if the user doesn't want to group processes and threads he won't even try to group them. But if decided to do so why should the IDE restrict him. Other debuggers (ours) don't show processes and threads. We don't want such restriction either. May be we will introduce container type and debugger can specify if groping is allow between different types. In any case specific debugger will have final saying if the group action is enabled or disabled so that won’t be an issue. I’d like to have generic implementation with restriction added at a higher level. What do you think? > > > > - The newly created group will be directly under the process node. > > > > - The process cannot be grouped. > > > Since multi-process is almost ready, I hope we can keep the design generic > > > enough that we will be able to easily include processes later. In fact, I'm > > > hoping for a grouping solution that allows to group anything, potentially > > > restricted to the elements of the group being of the same type. So we could > > > group threads together, or processes together, or groups together, or anything > > > else that is an IDMContext. > > OK. Will consider that. When can I see mutiple processes under the same launch? > Today :-) > Use GDB 7.2, launch a local session (attach or not) and then press the green > 'connect' button on the DV toolbar to attach to more processes. You will have > to start the processes outside eclipse first. > Note that DSF-GDB does support creating new processes in a running session but > I haven't added any UI to request that yet. So you can't try it out. Perfect. I'll start testing with that in too.
(In reply to comment #4) > I’d like to have generic implementation with restriction added at a higher > level. > > What do you think? That sounds good to me. I like that we would have less restrictions, I was just worried about the implementation. But we can see how that goes when we get there. > > > OK. Will consider that. When can I see mutiple processes under the same launch? > > Today :-) > > Use GDB 7.2, launch a local session (attach or not) and then press the green > > 'connect' button on the DV toolbar to attach to more processes. You will have > > to start the processes outside eclipse first. > > Note that DSF-GDB does support creating new processes in a running session but > > I haven't added any UI to request that yet. So you can't try it out. > > Perfect. I'll start testing with that in too. Thanks!
Created attachment 190097 [details] My first patch for the grouping feature. In order to try it out you need to: -Enable “Debug View Layout” command group in the debug perspective. -Start a GDB debug session with more than one running threads. -Select more than one thread in the debug view and then context menu “Group”. o See a use group is created for these threads. -If the program has more than few threads create few groups and group then together. -Or group few threads and few groups together. -When threads are halted they will automatically be expanded and selected. Supported debugger: -Windows Cygwin with GDB 6.8 – stop mode. -Windows MinGW with GDB 7.2 – stop mode. -Ubuntu 10.10 with GDB 7.2 – stop mode and non-stop mode. Known issue: -Ungroup is not implemented. -The thread and user group running state is not shown well in some case for stop mode debuggers. Next steps: -Implement "ungroup" and feedback from others. -Fix running state for the groups. -Refactor some of the code so it integrates against other debuggers (TI). -Test with Attach debugger to process – it will flush out the case with multiple top most containers.
(In reply to comment #6) > Created attachment 190097 [details] > My first patch for the grouping feature. This is really awesome! I didn't look at the patch itself, but I tested it and I really like the feel of the feature. It is very very exciting! > Known issue: > -Ungroup is not implemented. > -The thread and user group running state is not shown well in some case for > stop mode debuggers. I also saw this issue with non-stop. When I created a group of two of the three threads, the two threads were shown as running although they were stopped. We should probably remove empty groups. If I select all the threads from a group and a thread outside the group, and group them, a new group is created and the old one is left empty. > Next steps: > -Implement "ungroup" and feedback from others. Do you feel the patch is in a state to be reviewed, or is it still in a state where testing of the feature is more important? Again, I haven't looked at the patch, but do you think the feature is in a state where we could get it committed before March 11th (API freeze), if we work real hard at it. I really wasn't expecting it to make it for API freeze, but now that I see the feature already running, I wonder.
> I didn't look at the patch itself, but I tested it and I really like the feel > of the feature. It is very very exciting! Thanks for spending the time and giving a run :) > > -The thread and user group running state is not shown well in some case for > > stop mode debuggers. > I also saw this issue with non-stop. When I created a group of two of the > three threads, the two threads were shown as running although they were > stopped. Yes, this is #2 item in my TODO after hooking ungroup. Once I fix is for the stop mode I'll see if I can reproduce it with non-stop. > We should probably remove empty groups. If I select all the threads from a > group and a thread outside the group, and group them, a new group is created > and the old one is left empty. It might be a good thing to do. But I rather delay it a bit before we have a better idea what we want to do with persisting groups between debug sessions. It might be possible to have groups that are initially empty but as the user program runs it will dynamically create new threads that in the previous debug session belonged to that group. When we introduce hiding threads having empty groups may not be so strange. What about for a very first round I implement ungroup in a way that if we ungroup all items the group will be automatically removed? Also to remove the ambiguity of “Ungroup” on groups that belong to other groups I think I also need to introduce “Remove Group” menu item. > Do you feel the patch is in a state to be reviewed, or is it still in a state > where testing of the feature is more important? > Again, I haven't looked at the patch, but do you think the feature is in a > state where we could get it committed before March 11th (API freeze), if we > work real hard at it. I really wasn't expecting it to make it for API freeze, > but now that I see the feature already running, I wonder. I personally would like to have the code applied for March 11, assuming it doesn’t break anything. The feature is OFF by default, but whoever works on it should not need to re-apply the patch over and over. If we don’t have the patch in the main branch we have to constantly merge the head. What about after I finish the ungroup then we get it in? For now can you review it with a simple purpose to make sure it won’t break anything unless the feature is really used? Can we treat it as experimental feature?
(In reply to comment #8) > > We should probably remove empty groups. If I select all the threads from a > > group and a thread outside the group, and group them, a new group is created > > and the old one is left empty. > > It might be a good thing to do. But I rather delay it a bit before we have a > better idea what we want to do with persisting groups between debug sessions. > It might be possible to have groups that are initially empty but as the user > program runs it will dynamically create new threads that in the previous debug > session belonged to that group. I hadn't thought it through. I thought it was an over-site. Let's leave it the way you want and see where it goes. It really is a minor point. > I personally would like to have the code applied for March 11, assuming it > doesn’t break anything. That would be nice. It is just that the previous patches were really for testing (they faked groups), but if this is a real solution, then I can start reviewing it. > What about after I finish the ungroup then we get it in? > > For now can you review it with a simple purpose to make sure it won’t break > anything unless the feature is really used? > > Can we treat it as experimental feature? I'm not sure how that works, but if we can keep the feature turned off, I think it will be ok. I'll start reviewing it.
Created attachment 190219 [details] support for ungroup added This is a small addition on top of the previous patch that adds supports ungrouping. A group(s) has to be selected to remove it. The patch does not fire ContainerExitedDMEvent, but the final implementation should. Now I’m moving to TODO item #2.
Created attachment 190584 [details] make sure we don't enable any of the grouping code if the "Debug View Layout" action group is disabled.
Created attachment 190725 [details] Same patch simply updated for HEAD I had to update the patch for HEAD because I made a commit to LaunchVMProvider. (In reply to comment #11) > Created attachment 190584 [details] > make sure we don't enable any of the grouping code if the "Debug View Layout" > action group is disabled. I reviewed the plugins: debug.core, debug.ui, cdt.dsf and dsf.ui. I'm left with the two from DSF-GDB. I really like how things look! Nice work. It looks very generic. I only have some really minor comments. I'll review DSF-GDB tomorrow. Dobrin, can you split the patch into two? One with debug.core, debug.ui, cdt.dsf and dsf.ui, and the other with dsf.gdb and dsf.gdb.ui? If the DSF-GDB part is not ready, we could commit the first patch without having to wait. - Shall we make the action set global and rename to something like "C/C++ Advanced Debugging Features"? It can then be used for Pin&Clone. What do you think? Is that ok with Patrick? - in DMContexts, typo for variable 'hasAccrestorOfType" should be 'hasAncestorOfType' - IExecutionContextTranslator. I wonder if group() should use a DataRequestMonitor<IContainerDMContext>, so it can return the created group? Although the way we use it, we don't have a use for that yet. I guess we can leave as is since it is experimental and we can change it later - in dsf.ui, can you rename the new package 'org.eclipse.cdt.dsf.debug.internal.ui.debug.view.layout' to be 'org.eclipse.cdt.dsf.debug.internal.ui.debugview.layout' - in dsf.ui, can you also create package 'org.eclipse.cdt.dsf.debug.internal.ui.debugview.layout.actions' and put the new actions in there instead of in 'org.eclipse.cdt.dsf.debug.internal.ui.actions' - I think it would be better to have DsfDebugViewLayoutCommand implements IDebugCommandHandler. It actually indirectly does that through DsfGroupDebugContextsCommand and DsfUngroupDebugContextsCommand, but writing it explicitly gets the compiler to check things a little better, I think. - AbstractExecutionContextVMNode is a new public API. Should we mark it as Experimental? - the patch does not have the couple of updates of bug 240208 comment 49
Pawel, there are some backwards compatible changes to DSF in this patch. The plan is to commit them before Friday. All changes won't affect existing functionality.
Created attachment 190769 [details] Updated based on Mark's comments - CDT portion
Created attachment 190770 [details] Updated based on Mark's comments - GDB portion
> Dobrin, can you split the patch into two? One with debug.core, debug.ui, > cdt.dsf and dsf.ui, and the other with dsf.gdb and dsf.gdb.ui? If the DSF-GDB > part is not ready, we could commit the first patch without having to wait. Done. > - Shall we make the action set global and rename to something like "C/C++ > Advanced Debugging Features"? It can then be used for Pin&Clone. What do you > think? Is that ok with Patrick? I prefer finer granularity in customization. What if some products like one feature but not the other? Let’s have it like this until we figure what we want. > - in DMContexts, typo for variable 'hasAccrestorOfType" should be > 'hasAncestorOfType' Done. > - IExecutionContextTranslator. I wonder if group() should use a > DataRequestMonitor<IContainerDMContext>, so it can return the created group? > Although the way we use it, we don't have a use for that yet. I guess we can > leave as is since it is experimental and we can change it later I was puzzled with the same question I initially introduced the method. The problem is that we don’t have the use cases for the client other than the menu action. What if a group cannot be created, should we fail? What if a group can be created but exclude some of the context, etc. I prefer to have more defined API once we have good understanding of the user cases. Right now we have only one use case and the API fits it. What do you think? > - in dsf.ui, can you rename the new package > 'org.eclipse.cdt.dsf.debug.internal.ui.debug.view.layout' to be > 'org.eclipse.cdt.dsf.debug.internal.ui.debugview.layout' Done. > - in dsf.ui, can you also create package > 'org.eclipse.cdt.dsf.debug.internal.ui.debugview.layout.actions' and put the > new actions in there instead of in > 'org.eclipse.cdt.dsf.debug.internal.ui.actions' Done. > - I think it would be better to have DsfDebugViewLayoutCommand implements > IDebugCommandHandler. It actually indirectly does that through > DsfGroupDebugContextsCommand and DsfUngroupDebugContextsCommand, but writing it > explicitly gets the compiler to check things a little better, I think. Done. > - AbstractExecutionContextVMNode is a new public API. Should we mark it as > Experimental? Done. > - the patch does not have the couple of updates of bug 240208 comment 49 Done.
(In reply to comment #16) > > > - Shall we make the action set global and rename to something like "C/C++ > > Advanced Debugging Features"? It can then be used for Pin&Clone. What do you > > think? Is that ok with Patrick? > > I prefer finer granularity in customization. What if some products like one > feature but not the other? Let’s have it like this until we figure what we > want. ok > > - IExecutionContextTranslator. I wonder if group() should use a > > DataRequestMonitor<IContainerDMContext>, so it can return the created group? > > Although the way we use it, we don't have a use for that yet. I guess we can > > leave as is since it is experimental and we can change it later > > I was puzzled with the same question I initially introduced the method. The > problem is that we don’t have the use cases for the client other than the menu > action. What if a group cannot be created, should we fail? What if a group can > be created but exclude some of the context, etc. I prefer to have more defined > API once we have good understanding of the user cases. Right now we have only > one use case and the API fits it. > What do you think? Since it is marked Experimental, we can wait. > > - in dsf.ui, can you rename the new package > > 'org.eclipse.cdt.dsf.debug.internal.ui.debug.view.layout' to be > > 'org.eclipse.cdt.dsf.debug.internal.ui.debugview.layout' I think we need to export that package in the MANIFEST.MF file
(In reply to comment #14) > Created attachment 190769 [details] > Updated based on Mark's comments - CDT portion I'm ok with this patch. I still have to review the second though. However, since this first patch does not require the second, I suggest we commit it now. Patrick, can you take care of it to avoid the IPReview (that it is too late for anyway).
Update the copyright in couple files and set org.eclipse.cdt.dsf.debug.internal.ui.debugview.layout package to have hidden visibilty. Committed the CDT portion patch.
*** cdt cvs genie on behalf of pchuong *** Bug 336876 - [multicore] Grouping of Execution Elements for DSF debugger feature [*] DefaultVMModelProxyStrategy.java 1.16 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/dsf/org.eclipse.cdt.dsf.ui/src/org/eclipse/cdt/dsf/ui/viewmodel/DefaultVMModelProxyStrategy.java?root=Tools_Project&r1=1.15&r2=1.16 [+] DsfUngroupDebugContextsCommand.java http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/dsf/org.eclipse.cdt.dsf.ui/src/org/eclipse/cdt/dsf/debug/internal/ui/debugview/layout/actions/DsfUngroupDebugContextsCommand.java?root=Tools_Project&revision=1.1&view=markup [+] DsfGroupDebugContextsCommand.java http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/dsf/org.eclipse.cdt.dsf.ui/src/org/eclipse/cdt/dsf/debug/internal/ui/debugview/layout/actions/DsfGroupDebugContextsCommand.java?root=Tools_Project&revision=1.1&view=markup [+] DsfDebugViewLayoutCommand.java http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/dsf/org.eclipse.cdt.dsf.ui/src/org/eclipse/cdt/dsf/debug/internal/ui/debugview/layout/actions/DsfDebugViewLayoutCommand.java?root=Tools_Project&revision=1.1&view=markup [*] AbstractContainerVMNode.java 1.10 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/dsf/org.eclipse.cdt.dsf.ui/src/org/eclipse/cdt/dsf/debug/ui/viewmodel/launch/AbstractContainerVMNode.java?root=Tools_Project&r1=1.9&r2=1.10 [+] AbstractExecutionContextVMNode.java http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/dsf/org.eclipse.cdt.dsf.ui/src/org/eclipse/cdt/dsf/debug/ui/viewmodel/launch/AbstractExecutionContextVMNode.java?root=Tools_Project&revision=1.1&view=markup [*] AbstractThreadVMNode.java 1.14 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/dsf/org.eclipse.cdt.dsf.ui/src/org/eclipse/cdt/dsf/debug/ui/viewmodel/launch/AbstractThreadVMNode.java?root=Tools_Project&r1=1.13&r2=1.14 [*] plugin.xml 1.28 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/dsf/org.eclipse.cdt.dsf.ui/plugin.xml?root=Tools_Project&r1=1.27&r2=1.28 [+] DebugViewLayoutTester.java http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/dsf/org.eclipse.cdt.dsf.ui/src/org/eclipse/cdt/dsf/debug/internal/ui/debugview/layout/DebugViewLayoutTester.java?root=Tools_Project&revision=1.1&view=markup [*] MANIFEST.MF 1.20 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/dsf/org.eclipse.cdt.dsf.ui/META-INF/MANIFEST.MF?root=Tools_Project&r1=1.19&r2=1.20 [*] plugin.properties 1.126 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/all/org.eclipse.cdt.debug.ui/plugin.properties?root=Tools_Project&r1=1.125&r2=1.126 [*] plugin.xml 1.257 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/all/org.eclipse.cdt.debug.ui/plugin.xml?root=Tools_Project&r1=1.256&r2=1.257 [+] UngroupDebugContextsCommandHandler.java http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/all/org.eclipse.cdt.debug.ui/src/org/eclipse/cdt/debug/internal/ui/commands/UngroupDebugContextsCommandHandler.java?root=Tools_Project&revision=1.1&view=markup [+] GroupDebugContextsCommandHandler.java http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/all/org.eclipse.cdt.debug.ui/src/org/eclipse/cdt/debug/internal/ui/commands/GroupDebugContextsCommandHandler.java?root=Tools_Project&revision=1.1&view=markup [+] IUngroupDebugContextsHandler.java http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/all/org.eclipse.cdt.debug.core/src/org/eclipse/cdt/debug/core/model/IUngroupDebugContextsHandler.java?root=Tools_Project&revision=1.1&view=markup [+] IGroupDebugContextsHandler.java http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/all/org.eclipse.cdt.debug.core/src/org/eclipse/cdt/debug/core/model/IGroupDebugContextsHandler.java?root=Tools_Project&revision=1.1&view=markup [*] MANIFEST.MF 1.8 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/dsf/org.eclipse.cdt.dsf/META-INF/MANIFEST.MF?root=Tools_Project&r1=1.7&r2=1.8 [+] IExecutionContextTranslator.java http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/dsf/org.eclipse.cdt.dsf/src/org/eclipse/cdt/dsf/debug/internal/provisional/service/IExecutionContextTranslator.java?root=Tools_Project&revision=1.1&view=markup [*] AbstractDsfDebugServicesFactory.java 1.4 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/dsf/org.eclipse.cdt.dsf/src/org/eclipse/cdt/dsf/debug/service/AbstractDsfDebugServicesFactory.java?root=Tools_Project&r1=1.3&r2=1.4 [*] DMContexts.java 1.4 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/dsf/org.eclipse.cdt.dsf/src/org/eclipse/cdt/dsf/datamodel/DMContexts.java?root=Tools_Project&r1=1.3&r2=1.4
(In reply to comment #15) > Created attachment 190770 [details] > Updated based on Mark's comments - GDB portion This patch is very good as well. However, I think we still need to add quite a few things to get things working properly. My biggest concern is the fact that IContainerDMC used to be a process and we make that assumption all over the place. Since the patch does not change APIs (all new APIs are internal), and should not affect existing behavior, as long as the user does not enable the grouping, I don't think we should rush to commit before API freeze. I would prefer to get the issues fixed first, and commit it then. Below are some comments on the patch: - GdbUIPlugin: we need to shutdown ContextServiceListener to remove the ContextManagerListener - I think we need to update MIRunControl#makeExecutionDMCs to properly create the IExecutionDMC - GDBRunControl#createMIExecutionContext, I don't think we need to modify the container here since we call fProcService.createExecutionContext() which modifies the container again. Note that you didn't make that change in GDBRunControl_7_0_NS - GDBRunControl_7_0#createMIExecutionContext, I don't think we need to modify the container here since we call fProcService.createExecutionContext() which modifies the container again. Note that you didn't make that change in GDBRunControl_7_0_NS - In IMIContainerDMC, I suggest replacing private boolean isTopLevelContainer(IContainerDMContext containerDMC) { return containerDMC.getClass().toString().endsWith("GDBContainerDMC"); //$NON-NLS-1$ } with private boolean isTopLevelContainer(IContainerDMContext containerDMC) { return containerDMC instanceof IMIContainerDMContext; } - We need an interface IUserGroupDMContext with the method getId(). - UserGroupDMC should be made protected and IUserGroupDMContext used instead -> I think we have a problem when we use DMContexts.getAncestorOfType(dmc, IContainerDMContext.class) because the code was expecting to find a process, but now we can end up with a group. We will need to go through the code and replace that call with DMContexts.getTopMostAncestorOfType(dmc, IContainerDMContext.class) For example, if you have a group and you right click on a thread and choose Restart, it does not work. And when we will be able to group processes, getTopMostAncestorOfType() won't even work. We can use IMIContainerDMC instead, but that only work in DSF-GDB, not DSF itself, since it does know that type. - I think you've mentioned this before, and it is related to the previous point, but there are assumptions all over the place that an IContainerDMC is a process, while now it can be a group. We will have to look at all the uses of IContainerDMC
Created attachment 190953 [details] Updated GDB portion I updated "Updated based on Mark's comments - GDB portion" to match HEAD, and I made minor modifications to add 'private' in a bunch of places. Dobrin, you can do a diff between the two patches to see the difference, and if there is anything you don't like, please let me know.
(In reply to comment #22) > Created attachment 190953 [details] > Updated GDB portion > I updated "Updated based on Mark's comments - GDB portion" to match HEAD, and I > made minor modifications to add 'private' in a bunch of places. > Dobrin, you can do a diff between the two patches to see the difference, and if > there is anything you don't like, please let me know. OK by me. Thanks Mark.
Here is a discussion with Pawel and Mark on some of the design approaches I’ve tried: ------------------ Interesting find. I didn't intend isSuspended() to be applied on containers. I would suggest creating a separate asynchronous isSuspended() just for containers. Then use that in container label providers. My suggestion on grouping was to invent a new event when grouping is done and flush all the caches. I wouldn't overload started/exited because that's not what's reallly happening in the model. Cheers, Pawel P.S. I think it'd be better to discuss these on cdt-dev. Other people could contribute and benefit. On 04/01/2011 01:55 PM, Alexiev, Dobrin wrote: I wanted to ask you guys for ideas of how to show the suspended state of containers in the debug view: IRunControl.isSuspended(IExectionDMContext) is a synchronous API. At the same time IRunControl.getExecutionContexts() is an asynchronous API. So if I container wants to figure out the running state of its children it can’t do it in a synchronous manner. Looking at GDBRunControl_7_0_NS I see that: the service caches the running state when a thread is suspended. if a container is requested for its running state it checks in the cache is there is a suspended thread with a parent that matches the container if there is, that container is considered suspended. When I group an execution context, the object in the cache is no longer valid execution context – its parent have changed. Here is some options I can think of: Option 1: When the execution context is grouped it will fire exit event for that context and the cache entry will be removed. Then when the service is asked for the running state of the group - it has to ask all children for their suspended state. Somehow it has to update the list of the children synchronously and they refresh the data with their suspended state. In theory it is possibly because no new threads and processes are created, only a user group so the parent child relation can be obtained synchronously. Option 2: When the execution context is grouped it will fire exit event for that context and the cache entry will be removed. Then when the service is asked for the running state of the group - it has to ask all children for their suspended state. The new children are obtained asynchronously but the current suspended state is returned wrong. When the children sate is collected it somehow forces a parent update. Any other ideas? On a similar issue: When a non-stop GDB debugger list the treads of a give process it assumes that they are all suspended. Is that true. Is it possible some of them to be suspended when that are first read, so the cache of suspended threads is not really valid? Thanks Dobrin From: Alexiev, Dobrin Sent: Wednesday, March 30, 2011 4:19 PM To: 'Pawel Piech' Cc: Marc Khouzam Subject: RE: DSF question I think in that case components should reevaluate what has to be cached: Execution object or its underlying object (thread, process, core) This is also in the same direction if we need to hide / show execution context. If caches are based on thread id we can treat the execution objects transient. Yes, viewers need to be rebuilt. I think if we group two threads is OK to collapse them, as far as we leave the selection on the newly created group. In the case of hiding selected node – we just move the selection to parent or sibling. In a case of drastic layout change – I think ideally we’d like to select the node that was previously selected. I’ll keep digging…. Dobrin From: Pawel Piech [mailto:pawel.piech@windriver.com] Sent: Wednesday, March 30, 2011 2:33 PM To: Alexiev, Dobrin Cc: Marc Khouzam Subject: Re: DSF question Yep, if you do this, then you're entering into unchartered territory. If you do consider changing the parents, then I think you should cache contexts so that you only have one instance of a given context. I.e. you might as well do identity comparison on DM contexts. Flushing the caches is not going to be reliable IMO. For example, the viewer itself is another place where the element are cached and you have to flush the viewer using the viewer delta, which is very async. Another option is to say that if a context is moved to a group, it's a completely different context, and caches will just need to be rebuilt. Instead of using context-started/context-exited we could invent a new event like groups changed, which will tell the viewers to flush and refresh completely. The disadvantage there is that you'll have a hard time rebuilding the state of the Debug view after changing layout (expanded state and selection), though I don't know how important this is. Cheers, Pawel On 03/30/2011 11:20 AM, Alexiev, Dobrin wrote: That’s effectively breaking the promise that parents are part of the identity: Here is an instance of code that makes that assumption: public class GDBRunControl_7_0_NS extends AbstractDsfService implements IMIRunControl, ICachingService { public boolean isSuspended(IExecutionDMContext context) { // Thread case if (context instanceof IMIExecutionDMContext) { MIThreadRunState threadState = fThreadRunStates.get(context); return (threadState == null) ? false : !fTerminated && threadState.fSuspended; } // Container case. The container is considered suspended as long // as one of its thread is suspended if (context instanceof IContainerDMContext) { for (IMIExecutionDMContext threadContext : fThreadRunStates.keySet()) { if (DMContexts.isAncestorOf(threadContext, context)) { if (isSuspended(threadContext)) return true; } } } // Default case return false; } The execution context in the cache will have the original parents and will not be considered for computing the state. Even if I fix that I don’t know what else is relaying on the same assumption. I can see if flushing the cashes is the right approach. I’ll keep experimenting both ways. Other options? Dobrin From: Pawel Piech [mailto:pawel.piech@windriver.com] Sent: Wednesday, March 30, 2011 12:00 PM To: Alexiev, Dobrin Cc: Marc Khouzam Subject: Re: DSF question I see your point. In this case you can try to do some experiments. You can decouple the IDMContext.equals() from the full list of parents and do something more intelligent based on your knowledge of the model. If you do that, you can start to modify the parents list without affecting the equals() method. If you do that, then you should look at modifying the caching logic in the services so that it caches instances of DMContexts that are created and doesn't recreate them everytime. If you do that... you should do a lot of testing ;-) Cheers, Pawel On 03/30/2011 08:50 AM, Alexiev, Dobrin wrote: In our case some groups belongs to the back end. Issuing a Run control command to the backend on a group is different than looping thorough all execution units and running them. On another use case the layout of the debug view is dictated (to some level) by the back end and can change by the back end event. For example – I’d like to switch between: show the cores in terms of Master / Slave hierarchy, or in terms of JTAG hierarchy. I was under the impression that a IDMContext having multiple parents can give me good way to switch on the fly execution context and containers. Their parents ( threads, process) will say stable while the execution context and container context can live in a more dynamic environment. Do you think that if some objects ( groups) belong the back end or if the back end has some layout related functionality trying to play at the VMContext is a option to try out? Thanks Pawel From: Pawel Piech [mailto:pawel.piech@windriver.com] Sent: Wednesday, March 30, 2011 11:18 AM To: Alexiev, Dobrin Cc: Marc Khouzam Subject: Re: DSF question I haven't looked at the grouping feature implementation, but my first reaction is that grouping is a UI presentation concept not a model concept. So it should not change model elements, only position them differently. So if I was to implement grouping, I would create group objects which are only IVMContext objects and which do not contain a DMContext, etc. Cheers, Pawel On 03/30/2011 07:05 AM, Alexiev, Dobrin wrote: Pawel, When I implement groping in the debug view I’d like to move execution contexts around – effectively change their parents. For example: get two threads that originally had a process container as their parent and insert a user group between the threads and the process container. Since the set of parents is immutable inside AbstractDMContext effectively I need to recreate the new objects from the process down. Anyone that have cached that object needs to update its cache and point to the new execution object. So effectively I need to fire execution context exit event and they execution context started event with the new execution context. What is you recommendation for a proper implementation of this feature? Can you suggest different approaches to this? Thanks Dobrin
Created attachment 193874 [details] Updated GDB portion and new DSF changes Multiple changes: Set of supported features tested against GDB 7.2 ( both stop and non-stop mode) are listed at http://wiki.eclipse.org/CDT/MultiCoreDebugWorkingGroup/DebugElementsGrouping There are still outstanding items but the feature is much better shape than before. Design considerations are also summed up at: http://wiki.eclipse.org/CDT/MultiCoreDebugWorkingGroup/DebugElementsGrouping The patch contains new changes for CDT-DSF.
Thanks Dobrin! I skimmed through the changes and it looks pretty elegant. From what I'm guessing, you were able to fix the CommandCache problem by limiting the change to a new MICommandCache? As for the DSF change, I assume it is just the IRunControl3 class? I personally feel that having isStepping() and isSuspended() with an requestMonitor is a fine addition, no matter what happens to the grouping feature. Unless Pawel objects, I think we can commit that change, and it does not even have to be EXPERIMENTAL. After all, having IRunControl3 does not mean people have to use it. Are there other DSF changes that I didn't notice?
(In reply to comment #26) > Thanks Dobrin! > I skimmed through the changes and it looks pretty elegant. > From what I'm guessing, you were able to fix the CommandCache problem by > limiting the change to a new MICommandCache? Yes, that is my goal. For sure it worked for the varaibles and registers view. I looked at other uses of caches and looks like some of them are at the process - we don't need to change that yet ( not until we allow creating use groups as parents of processes nodes). Probably you can do better job here - deciding which cache should be converted to thread context, which shoud not? > As for the DSF change, I assume it is just the IRunControl3 class? I > personally feel that having isStepping() and isSuspended() with an > requestMonitor is a fine addition, no matter what happens to the grouping > feature. Unless Pawel objects, I think we can commit that change, and it does > not even have to be EXPERIMENTAL. After all, having IRunControl3 does not mean people have to use it. I think before we change to the new interface, we need to make sure all places in DSF that use the old functions (isSuspended() and isStepping()) should change to use the new one if the IRunControl3 is available. I didn't have chance to do all changes. Do you have some time to do it? I've been working lately 100% on the grouping so now I have to spend more time on our product. >>> Are there other DSF changes that I didn't notice? No, that is all chaged in this patch. But again - I only added one use of IRunControl3. We should do all of them, I think.
(In reply to comment #27) > I think before we change to the new interface, we need to make sure all places > in DSF that use the old functions (isSuspended() and isStepping()) should > change to use the new one if the IRunControl3 is available. > I didn't have chance to do all changes. Do you have some time to do it? I've > been working lately 100% on the grouping so now I have to spend more time on > our product. I have no time right now :-( But I'll keep it in mind as soon as some time frees up. Thanks for you efforts!
*** Bug 321925 has been marked as a duplicate of this bug. ***
Note sure if this was proposed before, but how about a concept of 'dynamic groups'? The user could create a group of 'running' threads, where each thread that is running would automatically put in the group and removed if the thread suspends. This would give an elegant way of hiding/grouping running threads as requested in Bug 375585. The idea would apply to running/suspended/crashed threads. 'Dynamic groups' could also be used to automatically show threads by cores, or by processes.
We're working on this feature again. We've evolving the design to try to address the following points: 1- support grouping threads of different processes 2- support grouping processes 3- default "Group-All" which represents all debug entities without groups (original representation). This group is read-only. This group is needed for different reasons: a) since threads can belong to many groups, we will create 'copies' of those threads, so the original threads must be kept in a place that makes sense: the group-all b) it will be easier to find a particular entry in group-all then in some group where that entry was put 4- nested groups (part of Dobrin's design) 5- Each group should be created as child of the top element. We might think of putting a group under a process, but this starts to fail of the group contains threads of multiple processes. 6- Sub-groups should also be children of top element (besides being part of its parent group) Since a group (like a thread) can belong to multiple groups, we need a place to put the original version of that group, which will be as a child of the top element 7- it would be nice to have two automatic and dynamic groups: suspended and running We could also think of an 'exited' group, but that group can quickly grow and is normally not interesting for a user. If a thread exits, it is a normal event that is part of execution; on the contrary, a thread that is interrupted is not normal, it is caused by the debugger, and the user should be aware of it. 8- we won't try to show cores as a debug entry. The handling of it still brings up too many questions and seems artificial, especially on Linux. 9- when creating a group of threads, the process node of each thread, will be shown (once per group). There should be a preference to show the process as text in the thread label instead, to save space. 10- groups can be created by selection in the Debug view. They could also be created using a text expression/filter/matcher 11- when selecting a process for a group, that will imply future threads of that process should be in the group. If selecting threads of a process explicitly for a group, will not include future-threads. If all-threads are selected, we may pop-up a dialog asking the user if they meant to select the process, and use that dialog to explain the difference. This dialog could be silenced ("don't show again" check box). One way to show if future threads will be included in a group could be to show the process node in that group, while not showing the process node explicitly when threads were selected instead of the whole process. 12- "future-thread" handling should be thought of in more detail
New Gerrit change created: https://git.eclipse.org/r/60562
Gerrit change https://git.eclipse.org/r/60562 was merged to [master]. Commit: http://git.eclipse.org/c/cdt/org.eclipse.cdt.git/commit/?id=c7a94541e507629d8eb7705b4a46077978289e7e
New Gerrit change created: https://git.eclipse.org/r/75553
New Gerrit change created: https://git.eclipse.org/r/75559
(In reply to Eclipse Genie from comment #34) > New Gerrit change created: https://git.eclipse.org/r/75553 (In reply to Eclipse Genie from comment #35) > New Gerrit change created: https://git.eclipse.org/r/75559 Those were mistakes that have been abandoned.
Hi everyone, I'm working at Intel, in the Developer Software Engineering group responsible for e.g. debugging tools (our team is the contributor for GDB at Intel). We have been developing GDB support for Intel oneAPI toolkit which is now in beta phase. oneAPI brings some major changes for GDB to be able to handle thousands of threads in parallel when we run the OpenCL or SYCL programs. This of course affects also Eclipse CDT which we should also enhance and this feature might be something we would need or would like to develop further. What is the current status of this feature? Would we be able to help? We would like to discuss what we are planning for GDB/Eclipse and how we could contribute. Regards, -Ari
I would like to work on the grouping feature. I tried with the GDB portion of the patches https://bugs.eclipse.org/bugs/attachment.cgi?id=193874&action=diff mentioned in https://bugs.eclipse.org/bugs/show_bug.cgi?id=336876#c25, but the UI doesn't seem to work properly. @jonah@kichwacoders.com Could you please guide me on where should I start?
(In reply to Vishnu Sarath from comment #38) > @jonah@kichwacoders.com Could you please guide me on where should I start? Nothing to add here to the information in the history of this bug and general DSF dev/debugging.
