Backport vDSO regression.

- Revert the makeinfo workaround from 7.8-27.fc21.
- Further 1.75x improvement of the interactive symbols lookup (Doug Evans).
This commit is contained in:
Jan Kratochvil 2014-10-27 15:47:21 +01:00
parent 862e0ed47f
commit 8095c33951
3 changed files with 163 additions and 25 deletions

View File

@ -78,49 +78,91 @@ index 055c87e..90bcd6d 100644
--17pEHd4RhPHOinZp--
http://sourceware.org/ml/gdb-patches/2014-10/msg00525.html
Subject: [patch 2/2] Accelerate lookup_symbol_aux_objfile 8x
--K8nIJk4ghYZn606h
http://sourceware.org/ml/gdb-patches/2014-10/msg00612.html
Subject: [patchv2 2/2] Accelerate lookup_symbol_aux_objfile 14.5x [Re: [patch 0/2] Accelerate symbol lookups 15x]
--vtzGhvizbBRQ85DL
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Hi,
On Wed, 22 Oct 2014 10:55:18 +0200, Doug Evans wrote:
> For example, the count of calls to dict_hash before/after goes from 13.8M to 31.
> I'm guessing one t hing we're doing here is coping with an artifact of
> dwz:
lookup_symbol_aux_objfile() processing is very ineffective. For each primary
symtab it searches it and also all its secondary symtabs. But that means that
secondary symtabs included in many primary symtabs get needlessly searched
many times during one lookup_symbol_aux_objfile() run.
During my simple test on non-DWZ file (./gdb itself) it went 3684->3484.
lookup_symbol_aux_objfile does not care in which primary/secondary symtab the
symbol is found.
The problem is that dict_cash(val) is called for the same val for each block
(== symtab).
On DWZ the saving is probably much larger as there are many more symtabs due
to DW_TAG_partial_unit ones.
> what was once one global block to represent the entire objfile is
> now N.
Without DWZ there are X global blocks for X primary symtabs for X CUs of
objfile. With DWZ there are X+Y global blocks for X+Y primary symtabs for
X+Y CUs where Y are 'DW_TAG_partial_unit's.
For 'DW_TAG_partial_unit's (Ys) their blockvector is usually empty. But not
always, I have found there typedef symbols, there can IMO be optimized-out
static variables etc.
> [I'm sure the patches help in the non-dwz case, but I suspect it's less.
> Which isn't to say the patches aren't useful.
> I just need play with a few more examples.]
I agree.
[patch 2/2] could needlessly performance-regress non-DWZ cases, therefore
I have put back original ALL_OBJFILE_PRIMARY_SYMTABS (instead of my
ALL_OBJFILE_SYMTABS) as it is perfectly sufficient. For the performance
testcase of mine:
Benchmark on non-trivial application with 'p <tab><tab>':
Command execution time: 4.215000 (cpu), 4.241466 (wall) --- both fixes with new [patch 2/2]
Command execution time: 7.373000 (cpu), 7.395095 (wall) --- both fixes
Command execution time: 13.572000 (cpu), 13.592689 (wall) --- just lookup_symbol_aux_objfile fix
Command execution time: 113.036000 (cpu), 113.067995 (wall) --- FSF GDB HEAD
That is additional 1.75x improvement, making the total improvement 26.8x.
No regressions on {x86_64,x86_64-m32,i686}-fedora21pre-linux-gnu in standard
and .gdb_index-enabled runs. Neither of the patches should cause any visible
behavior change.
Thanks,
Jan
--K8nIJk4ghYZn606h
--vtzGhvizbBRQ85DL
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline; filename="idxcache2.patch"
Content-Disposition: inline; filename="idxcache2doug.patch"
gdb/
2014-10-20 Jan Kratochvil <jan.kratochvil@redhat.com>
2014-10-23 Jan Kratochvil <jan.kratochvil@redhat.com>
* symtab.c (lookup_symbol_aux_objfile): Use ALL_OBJFILE_SYMTABS, inline
lookup_block_symbol.
diff --git a/gdb/symtab.c b/gdb/symtab.c
index c530d50..bc800ef 100644
index c530d50..da13861 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -1657,15 +1657,25 @@ lookup_symbol_aux_objfile (struct objfile *objfile, int block_index,
const struct block *block;
struct symtab *s;
- ALL_OBJFILE_PRIMARY_SYMTABS (objfile, s)
+ gdb_assert (block_index == GLOBAL_BLOCK || block_index == STATIC_BLOCK);
+
+ ALL_OBJFILE_SYMTABS (objfile, s)
ALL_OBJFILE_PRIMARY_SYMTABS (objfile, s)
{
+ struct dict_iterator dict_iter;
+
@ -145,5 +187,5 @@ index c530d50..bc800ef 100644
}
--K8nIJk4ghYZn606h--
--vtzGhvizbBRQ85DL--

View File

@ -1770,3 +1770,101 @@ gdb/testsuite/
--
1.9.3
commit 54fbc750b54271efb75ae11ce49f14c4234a9476
Author: Jan Kratochvil <jan.kratochvil@redhat.com>
Date: Thu Sep 18 08:21:40 2014 +0200
Fix regression for Linux vDSO in GDB (PR gdb/17407).
since
5979d6b69b20a8355ea94b75fad97415fce4788c
https://sourceware.org/git/?p=binutils-gdb.git;a=commitdiff;h=5979d6b69b20a8355ea94b75fad97415fce4788c
vdso handling
https://sourceware.org/ml/binutils/2014-03/msg00082.html
https://sourceware.org/ml/binutils/2014-04/msg00003.html
Message-ID: <A78C989F6D9628469189715575E55B230AA884EB@IRSMSX104.ger.corp.intel.com>
I get on
kernel-3.16.2-200.fc20.x86_64
https://koji.fedoraproject.org/koji/buildinfo?buildID=575860
attaching its vdso.bin.gz
GDB (FSF HEAD 5e43d46791c4c66fd83947a12d4f716b561a9103) regression:
reproducer:
./gdb -ex start ./gdb
actual result / FAIL:
Got object file from memory but can't read symbols: File truncated.
expected result / PASS:
<nothing>
or / PASS:
warning: Could not load shared library symbols for linux-vdso.so.1.
Do you need "set solib-search-path" or "set sysroot"?
That "warning: Could not load shared library..." is mostly harmless (it is
a bug in GDB), in the FAIL case it is not printed just because
bfd_check_format() fails there.
It seems logical to me this way when the 'size' parameter has been already
added.
Alan Modra:
I was wrongly thinking that the section headers were
always last when I wrote that code. (They are now! If you relink
that vdso with current binutils master you won't hit this problem, but
that of course doesn't help existing kernels.)
I do not see a regression for add-symbol-file-from-memory for libncurses.so.5
from the original thread above.
Start of section headers: 1080 (bytes into file)
Size of section headers: 64 (bytes)
Number of section headers: 13
Section header string table index: 8
Section Headers:
[Nr] Name Type Address Off Size ES Flg Lk Inf Al
[ 8] .fake_shstrtab STRTAB 0000000000000780 000780 000076 00 A 0 0 32
Program Headers:
Type Offset VirtAddr PhysAddr FileSiz MemSiz Flg Align
LOAD 0x000000 0x0000000000000000 0x0000000000000000 0x0012fe 0x0012fe R E 0x1000
size == 0x2000
shdr_end == 0x778 == 1080 + 13 * 64
high_offset == 0x12fe
else if (size >= shdr_end)
- high_offset = shdr_end;
+ high_offset = size;
But then 0x778 < 0x780 for "Section header string table index" so whole
bfd_check_format() fails because section headers were not cleared here:
/* If the segments visible in memory didn't include the section headers,
then clear them from the file header. */
if (high_offset < shdr_end)
bfd/ChangeLog
2014-09-18 Jan Kratochvil <jan.kratochvil@redhat.com>
PR gdb/17407
* elfcode.h (bfd_from_remote_memory): Use SIZE for HIGH_OFFSET.
### a/bfd/ChangeLog
### b/bfd/ChangeLog
## -1,3 +1,8 @@
+2014-09-18 Jan Kratochvil <jan.kratochvil@redhat.com>
+
+ PR gdb/17407
+ * elfcode.h (bfd_from_remote_memory): Use SIZE for HIGH_OFFSET.
+
2014-07-16 H.J. Lu <hongjiu.lu@intel.com>
* elf32-i386.c (elf_i386_plt_sym_val): Match PLT entry only for
--- a/bfd/elfcode.h
+++ b/bfd/elfcode.h
@@ -1749,7 +1749,7 @@ NAME(_bfd_elf,bfd_from_remote_memory)
headers. */
}
else if (size >= shdr_end)
- high_offset = shdr_end;
+ high_offset = size;
else
{
bfd_vma page_size = get_elf_backend_data (templ)->minpagesize;

View File

@ -26,7 +26,7 @@ Version: 7.8
# The release always contains a leading reserved number, start it at 1.
# `upstream' is not a part of `name' to stay fully rpm dependencies compatible for the testing.
Release: 28%{?dist}
Release: 29%{?dist}
License: GPLv3+ and GPLv3+ with exceptions and GPLv2+ and GPLv2+ with exceptions and GPL+ and LGPLv2+ and BSD and Public Domain and GFDL
Group: Development/Debuggers
@ -897,9 +897,6 @@ mv -f readline-doc readline/doc
%build
rm -rf %{buildroot}
# https://bugzilla.redhat.com/show_bug.cgi?id=1154436
export PERL5LIB="$PERL5LIB${PERL5LIB:+:}/usr/share/texi2html/lib/Unicode-EastAsianWidth/lib"
# Identify the build directory with the version of gdb as well as the
# architecture, to allow for mutliple versions to be installed and
# built.
@ -1130,10 +1127,6 @@ echo ====================TESTING END=====================
%endif
%install
# https://bugzilla.redhat.com/show_bug.cgi?id=1154436
export PERL5LIB="$PERL5LIB${PERL5LIB:+:}/usr/share/texi2html/lib/Unicode-EastAsianWidth/lib"
# Initially we're in the %{gdb_src} directory.
cd %{gdb_build}
rm -rf $RPM_BUILD_ROOT
@ -1350,6 +1343,11 @@ then
fi
%changelog
* Mon Oct 27 2014 Jan Kratochvil <jan.kratochvil@redhat.com> - 7.8-29.fc21
- Backport vDSO regression.
- Revert the makeinfo workaround from 7.8-27.fc21.
- Further 1.75x improvement of the interactive symbols lookup (Doug Evans).
* Mon Oct 20 2014 Jan Kratochvil <jan.kratochvil@redhat.com> - 7.8-28.fc21
- Accelerate interactive symbols lookup 15x.