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

Marc Khouzam
2009-11-09T17:16:23Z

> -----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