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] Use mmap instead of obstack_alloc for dwarf debug sections.


On Tue, Jun 16, 2009 at 12:19 PM, Tom Tromey<tromey@redhat.com> wrote:

> Thanks.  One point of order -- if you re-send a patch, re-send the
> ChangeLog entry too.  Not a big deal, but it makes things simpler when
> reviewing.  Thanks.

Will do, thanks.

> I saw in your original post that this patch helps on
> memory-constrained machines; your example was 2G of debug info on a 4G
> machine. I just want to make sure that this doesn't hurt other cases;
> I don't expect it would, but it is good to cross the t's...

I saw minor gain on 16GB machine.

I don't see how this could hurt: we aren't using any more memory by doing
mmap, we just cooperate with buffer cache instead of fighting with it.

>
> Paul> + int was_mmaped;
>
> I would spell this "was_mmapped" (two "p"s).

Fixed.

> Paul> + /* When debugging .o files, we may need to apply relocations; see
> Paul> +    http://sourceware.org/ml/gdb-patches/2002-04/msg00136.html .
> Paul> +    We never compress sections in .o files, so we only need to
> Paul> +    try this when the section is not compressed.  */
> Paul> +  retbuf = symfile_relocate_debug_section (abfd, sectp, buf);
>
> If we're using mmap then we seem to skip this relocation step.
> This seems like a bug.

That's what "&& (sectp->flags & SEC_RELOC) == 0)" check before mmap is for.
In symfile_relocate_debug_section():

  /* We're only interested in sections with relocation
     information.  */
  if ((sectp->flags & SEC_RELOC) == 0)
    return NULL;

I've added a comment to that effect.

> IIUC, this is an unusual case. So perhaps one fix would be to
> introduce a new symfile_debug_section_needs_relocate_p, then protect
> the mmap branch with that.

I can add symfile_debug_section_needs_relocate_p() if that's preferable.

> Paul> + gdb_assert (munmap ((void *)map_begin, map_length) == 0);
>
> Space after the ")" of the cast to void *.

Fixed.

Thanks,
-- 
Paul Pluzhnikov

2009-06-16  Paul Pluzhnikov  <ppluzhnikov@google.com>

       * dwarf2read.c (dwarf_info_section, dwarf_abbrev_section)
       (dwarf_line_section, dwarf_pubnames_section, dwarf_aranges_section)
       (dwarf_loc_section, dwarf_macinfo_section, dwarf_str_section)
       (dwarf_ranges_section, dwarf_frame_section,
dwarf_eh_frame_section): Removed.
       (dwarf2_resize_section): Likewise.
       (dwarf2_read_section): Now static, use bfd_mmap() if possible.
       (dwarf2_get_section_info): New function.
       (munmap_section_buffer): Likewise.
       (dwarf2_per_objfile_cleanup): Likewise.
       (section_is_p): Signature change.
       * dwarf2-frame.c (dwarf2_build_frame_info): Use
       dwarf2_get_section_info instead of dwarf2_read_section.

Attachment: gdb-dwarf2-mmap-20090616.txt
Description: Text document


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