This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

RE: [PATCH:MI] Return a subset of a variable object's children


> From: gdb-patches-owner@sourceware.org on behalf of Nick Roberts
> Sent: Sunday, April 27, 2008 7:14 AM
> 
> This patch allows -var-list-children to create just some of a variable object's
> children.  Following a discussion last year with Mark Khouzam, this is useful
> for displaying large arrays (and structures) where it would take too long to
> create all the children: it is only really necessary to create those that are
> visible on the screen and update as further become visible.  It also allows a

Cool!
To be fully transparent though, DSF does not yet have a way to deal with different
behavior of different GDBs, so we won't be using such improvements right away.
We stick to common behaviors, until we can handle differences cleanly.
But such improvements may prove useful in the future.

> step size (stride) other than one.

I'm not sure what the stride would be used for.  Maybe something like printing
all even indexes of an array for example?
In any case, it is a pretty simple addition, and no one is forced to use it,
so I'm only asking to understand better.

> 2008-04-27  Nick Roberts  <nickrob@snap.net.nz>
> 
>       * mi/mi-cmd-var.c (mi_cmd_var_list_children): Add options to select
>       a subset of children.
> 
>       * varobj.h (varobj_list_children): Declare new parameters.
> 
>       * varobj.c (struct varobj): New member current_children.
>       (varobj_list_children): Create any new varobjs for children
>       specified by mi_cmd_var_list_children.
>       (create_child): Add parameter real_index.  Use it.
> 


I have a concern about the ordering of children.
I think not having a constant ordering for the children could prove a problem.
For example, I think the algorithm proposed will fail if a child
varObj is deleted by the user.  I believe deleting a varObj inserts NULL
in its current position, however, the algo always inserts at the end, so
it will miss the available deleted entry.

Also, the double loop may prove to be slow for large number of children.

I was thinking that we could keep order of children as they are defined
(current behaviour) but not fill them all, until requested.
We could create the full Vector of children as is done now by

  while (VEC_length (varobj_p, var->children) < var->num_children)
    VEC_safe_push (varobj_p, var->children, NULL);

but only actually create the children that have been requested by the user.
I'm not sure how much efficiency there is by allocating the memory before hand?
Also, is there no way to grow the vector by more than a single point at a time?

We can even improve on that by doing the following:
instead of allocating the vector for all children, we can allocate the vector
for the children up to start+number*stride.

I believe this will give a constant ordering (same as current) and avoid the
costly double loop, since we can simply check for NULL to know if a child
is there or not.  And the delete will work as is.

You can also probably use the vector size instead of the new current_children.


> 
> *** mi-cmd-var.c      20 Apr 2008 10:20:39 +1200      1.50
> --- mi-cmd-var.c      27 Apr 2008 21:34:58 +1200
> *************** mi_cmd_var_list_children (char *command,
[...]
> !   if (argc == 1 || argc == 2)
> !     {
> !       /* Get varobj handle, if a valid var obj name was specified */
> !       if (argc == 1)
> !     var = varobj_get_handle (argv[0]);
> !       else
> !     var = varobj_get_handle (argv[1]);
> !       if (var == NULL)
> !     error (_("Variable object not found"));
> ! 
> !       if (argc == 2)
> !     print_values = mi_parse_values_option (argv[0]);
> !       else
> !     print_values = PRINT_NO_VALUES;
> ! 
> !       start = 0;
> !       number = varobj_get_num_children (var);
> !     }
>     else
> !     {
> !       int optind = 0;
> !       char *optarg;
> !       while (1)
> !     {
> !       int opt = mi_getopt ("mi_cmd_var_list_children",
> !                            argc, argv, opts, &optind, &optarg);

It would be nice to use the string -var-list-children instead of
mi_cmd_var_list_children, since it may be used to print an error to the user.

> !       if (optind >= argc)
> !     error (_("Missing VARNAME"));

It would be nice to have the full Usage printed here.
  
> !       if (optind < argc - 1)
> !     error (_("Garbage at end of command"));
> ! 
> !       var = varobj_get_handle (argv[optind]);
 
Here, you need the var == NULL check again:
    
      if (var == NULL)
        error (_("Variable object not found"));

Or you can extract it from the if above and put it outside the if/else


Thanks for getting this started.

Marc


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]