Bug 356273

Summary:	conserve memory by merging adjacent DiLoc entries in the debug info location table
Product:	[Developer tools] valgrind	Reporter:	Ivo Raisr <ivosh>
Component:	general	Assignee:	Julian Seward <jseward>
Status:	RESOLVED MOVED
Severity:	wishlist	CC:	groot, ivosh, philippe.waroquiers
Priority:	NOR
Version First Reported In:	unspecified
Target Milestone:	---
Platform:	Compiled Sources
OS:	Solaris
Latest Commit:		Version Fixed/Implemented In:
Sentry Crash Report:
Attachments:	merging in canonicaliseLoctab log file

Description Ivo Raisr 2015-12-04 13:19:56 UTC

As investigated in bug:
356044 - Dwarf line info reader misinterprets is_stmt register
there are some possible gains for conserving memory if adjacent DiLoc entries will be merged.
The best place for this is canonicaliseLoctab() in m_debuginfo/storage.c

Comment 1 Philippe Waroquiers 2015-12-09 21:27:54 UTC

Created attachment 95964 [details]
merging in canonicaliseLoctab

The attached patch adds merging logic in the canonicalise Loctab.
Tested on a big executable, this patch seems useless:
When activating the trace, we only see merging of 0 size entries (that will be cleaned up
in anycase) or failed merging due to max size reached.

So, unless we find a test case where significant merging is done, I suggest to close
this bug with WONTFIX.

Comment 2 Ivo Raisr 2015-12-12 20:51:59 UTC

I tried the patch and I see quite a lot of merged entries on Solaris 12:
...
addLoc merging previous: addr 0x7ffd0130c, size 18, line 343,  current: addr 0x7ffd0131e, size 28, line 343.
...

For simple /bin/true, I see ~3700 for "none" tool and ~4700 for "memcheck" tool.
See the attached log file.

Comment 3 Ivo Raisr 2015-12-12 20:52:20 UTC

Created attachment 96021 [details]
log file

Comment 4 Ivo Raisr 2015-12-12 20:53:05 UTC

Forgot to say that /bin/true + dependent libraries were compiled by Solaris Studio 12.4, not by gcc.

Comment 5 Philippe Waroquiers 2015-12-13 12:55:02 UTC

(In reply to Ivo Raisr from comment #2)
> I tried the patch and I see quite a lot of merged entries on Solaris 12:
Yes, for sure, I also see a lot of 'addLoc merging' (which are done during addLoc merging).
(NB: the changes around "addLoc merging" are just minor code cleanup, there is there
no functional change).

So, this patch is supposed to add merging in a second phase, producing traces
"canonicaliseLoctab merging"

Even on a big executable, I see only irrelevant "canonicaliseLoctab merging"  (i.e. either 0 size
or too big size) traces, that leads to no gain.

So, before committing this patch, I would like to see some real gains.

Comment 6 groot 2021-07-12 11:16:10 UTC

Valgrind lives elsewhere and has been rearranged -- I can't find the file the proposed patch would even apply to.