Bug 464103

Summary:	Enhancement: add a client request to DHAT to mark memory to be histogrammed
Product:	[Developer tools] valgrind	Reporter:	Paul Floyd <pjfloyd>
Component:	dhat	Assignee:	Julian Seward <jseward>
Status:	RESOLVED FIXED
Severity:	normal	CC:	mark, pjfloyd
Priority:	NOR
Version First Reported In:	3.21 GIT
Target Milestone:	---
Platform:	Other
OS:	Linux
Latest Commit:		Version Fixed/Implemented In:
Sentry Crash Report:
Attachments:	Initial version Example usage With initial count + convenience macros Added doc

Description Paul Floyd 2023-01-10 15:35:05 UTC

I often get DHAT results that refer to data structures larger than the default 1024 byte limit.

One fix is to recompile DHAT with a higher limit.

It would be nice to also have a client request, so that the guest code could be instrumented. The flow woule be then
1. Do a normal DHAT run to identify heap hotspots
2. If some are larger than 1K then instrument:

struct Foo *foo = malloc(sizeof(struct Foo));
DHAT_HISTOGRAM_MEMORY(foo, sizeof(struct Foo));

3. Recompile guest and rerun

Comment 1 Paul Floyd 2023-01-25 19:27:25 UTC

Created attachment 155638 [details]
Initial version

Seems to work OK

Comment 2 Paul Floyd 2023-01-25 19:28:52 UTC

Created attachment 155639 [details]
Example usage

Comment 3 Paul Floyd 2023-01-25 20:48:49 UTC

Created attachment 155641 [details]
With initial count + convenience macros

Comment 4 Paul Floyd 2023-04-21 19:06:49 UTC

Created attachment 158281 [details]
Added doc

Comment 5 Mark Wielaard 2023-04-21 19:20:38 UTC

Comment on attachment 158281 [details]
Added doc

Looks good to me. But I don't have much experience with dhat myself. So it would be good to get some feedback from someone else who does. I would say push it for RC2 and then we'll explicitly ask for feedback.

Comment 6 Mark Wielaard 2023-04-21 20:47:18 UTC

user_histo1.stdout.exp is missing from EXTRA_DIST, but that is easy to fix

On my local system the test however fails
--- user_histo1.stderr.exp      2023-04-21 22:33:09.538844963 +0200
+++ user_histo1.stderr.out      2023-04-21 22:38:58.353674798 +0200
@@ -1,8 +1,8 @@
Warning: request for user histogram of size 500 is smaller than the normal histogram limit, request ignored
-Warning: address for user histogram request not found 55b2820
+Warning: address for user histogram request not found 5b4e440
 Warning: request for user histogram of size 100000 is larger than the maximum user request limit, request ignored
-Total:     102,500 bytes in 3 blocks
-At t-gmax: 100,500 bytes in 2 blocks
+Total:     175,204 bytes in 4 blocks
+At t-gmax: 173,204 bytes in 3 blocks
 At t-end:  0 bytes in 0 blocks
 Reads:     1,000 bytes
-Writes:    102,520 bytes
+Writes:    102,536 bytes

the address should probably be filtered
I don't know what to do with the differing bytes and blocks though, I assume that depends on the libc used?

Comment 7 Paul Floyd 2023-04-27 08:59:04 UTC

commit 424340403c5d9eab23e883f6d27f780243bf8435
Author: Paul Floyd <pjfloyd@wanadoo.fr>
Date:   Fri Apr 21 21:21:23 2023 +0200

    Bug 464103 - Enhancement: add a client request to DHAT to mark memory to be histogrammed

(plus a few more pushes to cleanup and add feedback from Nick Nethercote).