From: Paul Floyd Date: Thu, 27 Apr 2023 07:19:46 +0000 (+0200) Subject: dhat: remove initial count value from access count histogram user requests X-Git-Tag: VALGRIND_3_21_0~4 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=be26a1773eeea9199911600135e22e5f0d0a3541;p=thirdparty%2Fvalgrind.git dhat: remove initial count value from access count histogram user requests Based on feedback from Nick Nethercote. --- diff --git a/NEWS b/NEWS index d9e04ab823..22d5d4a1d8 100644 --- a/NEWS +++ b/NEWS @@ -135,8 +135,7 @@ AMD64/macOS 10.13 and nanoMIPS/Linux. * DHAT: - A new kind of user request has been added which allows you to override the 1024 byte limit on access count histograms for blocks - of memories. Two client requests are available, - DHAT_HISTOGRAM_MEMORY_UNINIT and DHAT_HISTOGRAM_MEMORY_INIT. + of memory. The client request is DHAT_HISTOGRAM_MEMORY. * ==================== FIXED BUGS ==================== diff --git a/dhat/dh_main.c b/dhat/dh_main.c index ac0a7478e9..24d1c2768b 100644 --- a/dhat/dh_main.c +++ b/dhat/dh_main.c @@ -1232,7 +1232,6 @@ static Bool dh_handle_client_request(ThreadId tid, UWord* arg, UWord* ret) case VG_USERREQ__DHAT_HISTOGRAM_MEMORY: { Addr address = (Addr)arg[1]; - UWord initial_count = arg[2]; Block* bk = find_Block_containing( address ); // bogus address @@ -1254,7 +1253,7 @@ static Bool dh_handle_client_request(ThreadId tid, UWord* arg, UWord* ret) return False; } - // already histogrammed + // too big if (bk->req_szB > USER_HISTOGRAM_SIZE_LIMIT) { VG_(message)( Vg_UserMsg, @@ -1266,13 +1265,8 @@ static Bool dh_handle_client_request(ThreadId tid, UWord* arg, UWord* ret) bk->histoW = VG_(malloc)("dh.new_block.3", bk->req_szB * sizeof(UShort)); - if (initial_count == 0U) { - VG_(memset)(bk->histoW, 0, bk->req_szB * sizeof(UShort)); - } else { - for (SizeT i = 0U; i < bk->req_szB; ++i) { - bk->histoW[i] = 1U; - } - } + VG_(memset)(bk->histoW, 0, bk->req_szB * sizeof(UShort)); + return True; } diff --git a/dhat/dhat.h b/dhat/dhat.h index cbedbe88b6..97e1ade8fc 100644 --- a/dhat/dhat.h +++ b/dhat/dhat.h @@ -65,7 +65,6 @@ typedef enum { VG_USERREQ__DHAT_AD_HOC_EVENT = VG_USERREQ_TOOL_BASE('D', 'H'), VG_USERREQ__DHAT_HISTOGRAM_MEMORY, - VG_USERREQ__DHAT_HISTOGRAM_ARRAY, // This is just for DHAT's internal use. Don't use it. _VG_USERREQ__DHAT_COPY = VG_USERREQ_TOOL_BASE('D','H') + 256 @@ -78,21 +77,10 @@ typedef VALGRIND_DO_CLIENT_REQUEST_STMT(VG_USERREQ__DHAT_AD_HOC_EVENT, \ (_qzz_weight), 0, 0, 0, 0) -// for limited histograms of memory larger than 1k -#define DHAT_HISTOGRAM_MEMORY(_qzz_address, _qzz_initial_count) \ +// for access count histograms of memory larger than 1k +#define DHAT_HISTOGRAM_MEMORY(_qzz_address) \ VALGRIND_DO_CLIENT_REQUEST_STMT(VG_USERREQ__DHAT_HISTOGRAM_MEMORY, \ - (_qzz_address), (_qzz_initial_count), 0, 0, 0) - -// convenience macro for DHAT_HISTOGRAM_MEMORY -// for initialized memory (calloc, std::vector with initialization) -#define DHAT_HISTOGRAM_MEMORY_INIT(_qzz_address) \ - DHAT_HISTOGRAM_MEMORY(_qzz_address, 1U) - -// convenience macro for DHAT_HISTOGRAM_MEMORY -// for uninitialized memory (malloc, std::vector without initialization) -#define DHAT_HISTOGRAM_MEMORY_UNINIT(_qzz_address) \ - DHAT_HISTOGRAM_MEMORY(_qzz_address, 0U) - + (_qzz_address), 0, 0, 0, 0) #endif diff --git a/dhat/docs/dh-manual.xml b/dhat/docs/dh-manual.xml index 9d99b4f352..8f30ff7f7e 100644 --- a/dhat/docs/dh-manual.xml +++ b/dhat/docs/dh-manual.xml @@ -427,24 +427,22 @@ to 1024 bytes. This is done to limit the performance overhead and also to keep the size of the generated output reasonable. However, it is possible to override this limit using client requests. The use-case for this is to first run DHAT normally, and then identify any large blocks that you would like to further -investigate with access histograms. The client requests are declared in -dhat/dhat.h. There are two versions, DHAT_HISTOGRAM_MEMORY_INIT and -DHAT_HISTOGRAM_MEMORY_UNINIT. The -UNINIT version should be used with allocators that return uninitialized -memory (like malloc and new -without a constructor). The INIT version should be used with allocators -that initialize memory (like calloc and new -with a constructor). The macros should be placed immediately after the -call to the allocator, and use the pointer returned by the allocator. +investigate with access count histograms. The client request is declared in +dhat/dhat.h and is called DHAT_HISTOGRAM_MEMORY. +The macro should be placed immediately after the call to the allocator, +and use the pointer returned by the allocator. The memory that can be profiled in this way with user requests -has a further upper limit of 25kbytes. +has a further upper limit of 25kbytes. Be aware that the access counts +will all be set to zero. This means that the access counts will not +include any reads or writes performed during initialisation. An example where this +will happen are uses of C++ new with user-defined constructors. Access counts can be useful for identifying data alignment holes or other layout inefficiencies. diff --git a/dhat/tests/user_histo1.cpp b/dhat/tests/user_histo1.cpp index f5c8e69bbc..191fde2c5a 100644 --- a/dhat/tests/user_histo1.cpp +++ b/dhat/tests/user_histo1.cpp @@ -7,7 +7,7 @@ int main() { std::vector vec(2000, 0); - DHAT_HISTOGRAM_MEMORY_INIT(vec.data()); + DHAT_HISTOGRAM_MEMORY(vec.data()); std::mt19937 gen(42);; std::uniform_int_distribution<> index_distrib(0, 1999); std::uniform_int_distribution<> val_distrib(0, 255); @@ -23,12 +23,12 @@ int main() // try to generate some warnings vec.resize(500); vec.shrink_to_fit(); - DHAT_HISTOGRAM_MEMORY_UNINIT(vec.data()); + DHAT_HISTOGRAM_MEMORY(vec.data()); auto old = vec.data(); vec.resize(100000); // old should have been deleted - DHAT_HISTOGRAM_MEMORY_UNINIT(old); + DHAT_HISTOGRAM_MEMORY(old); // and this is too big - DHAT_HISTOGRAM_MEMORY_UNINIT(vec.data()); + DHAT_HISTOGRAM_MEMORY(vec.data()); }