Breakpoint Reset Project

1. Motivation

[For pretty pictures on the potential impact of this project, see below.]

Many inferior events require GDB to reset breakpoint locations. For example, whenever a new shared library's symbols are loaded, GDB must run through all existing locations to determine if any new locations need to be added to the breakpoint.

This is currently accomplished via a call to breakpoint_re_set. This function loops over the breakpoint table, calling breakpoint_re_set_one to "reset" (more correctly, "reparse") the location specified by the user.

The problem here is that for debug environments with large number of inferiors, breakpoints, and/or shared libraries, the sheer number of resets required becomes quite large. Since the current breakpoint API recreates every location every single time, every reset event creates a lot of unnecessary work.

For example, when a shared library is loaded, there is no need to investigate new breakpoint locations in objfiles outside of the shared library -- they have not changed.

The purpose of this project is to redesign/implement the breakpoint reset API so that it may do this time-consuming work more intelligently, thereby reducing the time needed to respond to various inferior events.

2. Design

I am proposing to change the breakpoint reset API to include a new parameter which describes why the breakpoint reset is necessary.

There are eleven direct callers breakpoint_re_set (event generators):

  1. breakpoint.c: enable_breakpoints_after_startup (exported)

    • Called by solib-spu.c:spu_current_sos.

  2. infcmd.c:post_create_inferior (exported)

    • Called by corelow.c:core_open, infcmd.c:run_command_1 (called by run_command and start_command), infcmd.c:attach_command_post_wait (called by attach_command_continuation, attach_command, notice_new_inferior)

  3. infrun.c:follow_inferior_reset_breakpoints

    • Called by follow_fork

    3b.related, in follow_fork itself, the way we pass the step_resume_breakpoint

    • from parent to child etc. is fragile. We're probably missing moving a bunch of stuff over. Can certainly not be handled now, but something to keep in mind.

  4. infrun.c:follow_exec

    • Called by handle_inferior_event

  5. objfiles.c:objfile_relocate (exported)

    • Called by remote.c:get_offsets (called by remote_start_remote and extended_remote_create_inferior)

  6. objfiles.c:objfile_rebase (exported)

    • Called by solib-darwin.c:darwin_solib_create_inferior_hook

  7. solib.c:solib_add (exported)

    • Called by infcmd.c:post_create_inferior, solib.c:sharedlibrary_command, solib.c:handle_solib_event, solib.c:reload_shared_libraries (immediately followed by a call to breakpoint_re_set), solib-frv.c:frv_fetch_objfile_link_map (called by frv-tdep.c:frv_gdbarch_init), solib-ia64-hpux.c:ia64_hpux_handle_dld_breakpoint_1, solib-irix.c:irix_solib_create_inferior_hook, solib-osf.c:osf_solib_create_inferior_hook, solib-svr4.c:svr4_fetch_objfile_link_map, solib-svr4.c:enable_break

  8. solib.c:reload_shared_libraries (immediately followed by call to

    • breakpoint_re_set)

  9. symfile.c:new_symfile_objfile (exported)

    • Called by symbol_file_add_with_addrs

  10. symfile.c:generic_load (exported)

    • Called by m32r-rom.c:m32r_load_gen and remote.c:remote_load

  11. symfile.c:clear_symtab_users (exported)

    • Called by m32r-rom.c:m32r-load, objfiles.c:free_all_objfiles (exported), remote-m32r-sdi.c:m32r_load, solib-som.c:som_solib_create_inferior_hook,

    • symfile.c:new_symfile_objfile, symfile.c:remove_symbol_file_command, symfile.c:reread_symbols (exported), symfile.c:clear_symtab_users_cleanup

      • (called by reread_symbols and syms_from_objfile_1)

Many of these can be broadly categorized (list is incomplete):

  1. New objfile events: breakpoint_re_set is being called because a new

    • obfile(s) was loaded by the inferior and gdb must check to see if it needs to add any new locations to existing breakpoints.
  2. fork-/clone-events: breakpoint_re_set is being called because the inferior

    • forked or cloned. It might be possible to short-circuit breakpoint_re_set entirely by simply copying breakpoints from the parent into the child/clone.

  3. unload events (degenerate case of #1?). An objfile was unloaded.
    • We need only remove/disable breakpoint locations in the unloaded objfile(s).

The proposed API change comprises modifications to both the internal breakpoint and the location parsing APIs.

Consider the case of an objfile load (example from solib_add):

  struct breakpoint_reset_reason reason;

  init_breakpoint_reset_reason (&reason);
  reason.reason = BREAKPOINT_RESET_ADD_OBJFILE;

  /* ... */
  VEC_safe_push (objfilep, reason.objfile_list, objfile);
  /* ... */
  breakpiont_re_set (&reason);

This notifies breakpoint_re_set that the reset is being done because new objfiles have been loaded by the inferior. This information can be then be used by location processing (nee linespec processing) to limit the scope of the work being done.

This is accomplished by adding a new parameter to the breakpoint method decode_location:

  struct decode_line_limits limits;

  limits.objfiles = reason->objfile_list;
  TRY
  {
    b->ops->decode_location (b, location, limits, &sals);
  }
  /* ... */

These limits will be passed down into location processing, which will then only search the given objfiles for new locations.

The are numerous problems to overcome in linespec.c to implement this, but the only design decision worth mentioning is how new locations are determined.

After trying (unsuccessfully) several different algorithms, it became necessary to add an additional requirement to this design: no symbol searching may be performed. Given this restriction, which arises with the need to deal with matching minimal_symbols of various classifications (see classify_mtype in linespec.c), I have decided to maintain the most generic caching implementation possible.

I have chosen to cache the result of location "parsing," i.e., the struct linespec passed to convert_location_to_sals. In order to do this, two small but significant (internal) API changes were necessary.

First, search_minsyms_for_name was modified to push all minimal symbols into the result list. It does no priority sorting of the symbols whatsoever.

Instead this priority sorting/elimination is done in convert_linespec_to_sals, merging it with the program space "filtering" that is already done there. When SaLs are created from the (cached) struct linespec, convert_linespec_to_sals will have complete access to all matching symbols and minimal symbols. The logic for symbol elimination is codified into a single location and used with or without limits. The breakpoint API itself has no knowledge of this internal implementation.

This linespec/location cache is maintained in the breakpoint itself and passed opaquely to the linespec/location processing.

3. Current Status

The basic APIs and infrastructure/refactoring is in place. The only event implementing this new design is solib_add, where new objfiles are added to the inferior. [Other add objfile events could fairly quickly be implemented, but the first milestone is to have this first optimization pass testing with no regressions.]

The current code base regresses info-shared.exp, unload.exp, and watchpoint-solib.exp. Many of these deal with the unloading of objfiles which has not yet been implemented. This will require the ability to loop through the breakpoint's linespec cache and remove locations set in the objfile list.

Note that I have littered the code with TODOs and other personal notes in comments prefixed with !!keiths.

4. Branch

The work is hosted in GDB's sourceware.org GIT repository on the branch "users/keiths/intelligent-breakpoint_re_set". This is a work in progress, and the design and/or implementation is subject to change.

5. Potential Impact

While these results are not going to be truly indicative of real-world impact of the project, they represent "best case" scenarios for one specific/common event case. Nonetheless, some quantification of the potential impact is enlightening.

I chose to quantify the impact of solib_add events in a completely native configuration. It does not attempt to measure the impact of any other type of breakpoint-reset event or the impact of anything with remote, simulator, or any other target. Because I completely skipped solib_add events, this represents a really best case scenario, since we really cannot completely skip these events entirely, e.g., we still need to check for pending breakpoints, new locations to existing breakpoints, etc.

How did I collect this data? Tests used rhythmbox with full debuginfo (on Fedora 21, x86_64) and tree-built GDB. In both cases, the approach was the same:

1. Create lists of breakpoints. To do this, I arbitrarily picked 0, 5, 10, 25, 50, 100, 250, 500, 750, 1000, 1250, 1500, 2000, and 2500 breakpoints. These were generated by doing "nm -C XXX | head -$bpnum > BPFILE", where XXX = gdb,libwebkitgtk-3.0.so. Some end up pending, most are resolved.

2. Collect the average time to start gdb, run to main, and create all breakpoints. (gdb -nx -q -batch -x BPFILE XXX -ex "b main" -ex r).

3. Do the same but now re-run the inferior. This will cause several breakpoint_re_set events to occur (solib_add, post_create_inferior, new_symfile_objfile, ...).

4. Repeat #2 and #3 but with a gdb modified to skip solib_add events. The gdb case skipped ALL solib_add events. The rhythmbox case skipped all non-libwebkit.so solib_add events (all breakpoints were set in that library).

5. All graphed with gnuplot. See the attached pretty pictures.

Now assuming that this methodology is correct enough to draw conclusions, it is pretty obvious that this could have a large impact on the user experience.

bpreset-rhythmbox-scaling.png bpreset-gdb-scaling.png

None: BreakpointReset (last edited 2016-05-24 10:18:09 by PedroAlves)

All content (C) 2008 Free Software Foundation. For terms of use, redistribution, and modification, please see the WikiLicense page.