RE: [MI] Extending -list-thread-groups --available to show cores

[Marc Khouzam <marc.khouzam@ericsson.com>]

> -----Original Message-----
> From: Vladimir Prus [mailto:vladimir@codesourcery.com] 
> Sent: Monday, November 09, 2009 11:29 AM
> To: Marc Khouzam
> Cc: 'gdb@sources.redhat.com'
> Subject: Re: [MI] Extending -list-thread-groups --available 
> to show cores
> 
> On Monday 09 November 2009 Marc Khouzam wrote:
> 
> > > > > We were recently asked to slightly extend the 
> returned information
> > > > > to include the core where each thread runs. Such 
> information is
> > > > > of little use for typical Linux application, since threads are
> > > > > migrated between cores. However, it's useful for both custom 
> > > > > Linux applications that specifically pin threads to 
> > > specific cores,
> > > > > and for embedded systems. Therefore, I plan to add a new field
> > > > > to the thread information that is output by 
> > 
> > Is there currently thread information in the output of 
> "--available"?
> 
> No.

Sorry, I had not understood from the spec that you would be adding
thread information to the output to "--availabe"

> > > > > -list-thread-groups --available, named 'core' that 
> will give the
> > > > > number of the core. E.g.
> > > > 
> > > > I assume you didn't mean to restrict this output to the 
> > > "--available"
> > > > form of "-list-thread-groups", but meant to say that it 
> would affect
> > > > all forms of "-list-thread-groups", right?
> > > 
> > > I actually did mean to restrict to --available ;-) But if 'core'
> > > will be beneficial for ordinary '-list-thread-group', 
> please assume
> > > it's there.
> > 
> > It is just that in the original email, the examples you gave were
> > not for the "--available" case :-)
> > 
> > 	-list-thread-groups
> > 	
> ^done,groups=[{id="17",type="process",pid="yyy",num_children="
> 2",cores=[1,2]}]
> > 	-list-thread-groups 17
> > 	^done,threads=[{id="2",target-id="Thread 0xb7e14b90 
> (LWP 21257)",cores=[1]
> > 	   
> frame={level="0",addr="0xffffe410",func="__kernel_vsyscall",ar
> gs=[]},state="running"},
> 
> I've accidentally left out --available; it should be there.

Ok.
But that means you are also proposing to support:
-list-thread-groups --available [group]

I'm just clarifying 'cause that is not suported today.
If fact, you are also suggesting
-list-thread-groups --available [group1] [group2] ...
right?
are you also suggesting 
-list-thread-groups [group1] [group2] ...
or not?
 
> > > Yes, "types" should be "type". Basically, we have a 
> > > compatibility issue
> > > here. Now, -list-thread-groups 17 prints only threads in 
> that process.
> > > And if we make '-list -thread-groups 17 18' print only 
> threads in one
> > > list, there will be no way to figure what process each thread 
> > > belongs to.
> > > We can either:
> > > 
> > > 1- add 'process' parent link to each thread
> > > 2- show groups, with threads inside them, as the above 
> output shows
> > > 
> > > The second approach seems easier for frontend, since it won't 
> > > be required
> > > to group threads itself. But it makes the output for '17' 
> and '17 18'
> > > cases be different in structure, so a frontend should be 
> prepared to
> > > both outputs. Does not seem like we can do much better?
> > 
> > What about #1 and having multiple "threads=", one for each process?
> >  Something like:
> > 
> > 	-list-thread-groups 17 18
> > 	^done,threads=[{id="2",group="17", target-id="Thread 
> 0xb7e14b90 (LWP 21257)",cores=[1]
> > 	   
> frame={level="0",addr="0xffffe410",func="__kernel_vsyscall",ar
> gs=[]},state="running"}}],
> > 		threads=[{id="3",group="18", target-id="Thread 
> 0xb7e14b90 (LWP 21257)",cores=[1]
> > 	   
> frame={level="0",addr="0xfffff410",func="__kernel_vsyscall",ar
> gs=[]},state="running"}}] 
> > 
> > This would make "-list-thread-groups 17" only get new 
> backwards-compatible fields,
> > while allowing "-list-thread-groups 17 18" to show threads 
> as part of a grouping.
> > Does this go against the rules of MI? 
> 
> While there's no explicit rule that names of fields are 
> unique, having them
> non-unique sounds a bit hacky to me. E.g. KDevelop parser 
> would not even
> be able to access such fields.

But even if a frontend does not support this format now, 
it is still a  backwards compatible solution since having
non-unique fields would only occur in this case when using
the new multiple-arg form of -list-thread-groups.

Would it be hard to have this concept supported by KDevelop?
I didn't try it in DSF-GDB, but since we loop over all fields,
each field, unique or not, should eventually be accessed, so it
should work quite easily.

One could argue that if a frontend cannot handle non-unique
fields, it should limit itself to issuing multiple
-list-thread-groups <group>
and not use the new
-list-thread-groups <group> ...

> > > Well, we probably can declare that -list-thread-groups is 
> so new that
> > > we can break backward compatibility -- what do you think?
> > 
> > This is tempting.  However, even if no other frontend is 
> using this now,
> > if a frontend wants to support GDB 7.0 and the next GDB, they would 
> > need to code for both outputs.  Keeping the output 
> backwards compatible 
> > will allow future frontends that don't want to use mutliple 
> parameters
> > to -list-thread-groups to have one way of parsing the output.
> 
> Then, maybe we should trick to the output I have originally suggested.
> It looks like having the frontend recognize both 'groups' and 
> 'threads'
> as top-level element in response is just as good as having duplicate
> field names. What do you think?

The reason I prefer duplicate field names is that I don't really like
the idea of having
-list-thread-groups group1
have a different output format than
-list-thread-groups group1 group2

In my opinion, if it is possible, the two should have the same format,
as long as backwards-compatibility can be preserved.

But maybe that is just me?

Marc