dhat: remove initial count value from access count histogram user requests

Based on feedback from Nick Nethercote.

diff --git a/NEWS b/NEWS
index d9e04ab823d64f1fc072fa37f19ca46191eaa9e3..22d5d4a1d8ddf46d0050bbdb18c170326ac65aad 100644 (file)
--- 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 ac0a7478e996d2815fcbcb66db4507a8a2d2d9f2..24d1c2768b3de3e1fb3e00861b42c215eff7dd8b 100644 (file)
--- 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 cbedbe88b6f35e849349c403aaaa38ecf0f2146c..97e1ade8fc52ae6d93fbdde5a65d014c5b447952 100644 (file)
--- 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 9d99b4f3525353ad41be4d02bc78c962845b9ece..8f30ff7f7ec9cefc2973f1acfbd47417246f4a9b 100644 (file)
--- 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
-<filename>dhat/dhat.h</filename>. There are two versions, <computeroutput>DHAT_HISTOGRAM_MEMORY_INIT</computeroutput> and
-<computeroutput>DHAT_HISTOGRAM_MEMORY_UNINIT</computeroutput>. The
-UNINIT version should be used with allocators that return uninitialized
-memory (like <computeroutput>malloc</computeroutput> and <computeroutput>new</computeroutput>
-without a constructor). The INIT version should be used with allocators
-that initialize memory (like <computeroutput>calloc</computeroutput> and <computeroutput>new</computeroutput>
-with a constructor). The macros should be placed immediately after the
-call to the allocator, and use the pointer returned by the allocator.</para>
+investigate with access count histograms. The client request is declared in
+<filename>dhat/dhat.h</filename> and is called <computeroutput>DHAT_HISTOGRAM_MEMORY</computeroutput>.
+The macro should be placed immediately after the call to the allocator,
+and use the pointer returned by the allocator.</para>
 
 <programlisting><![CDATA[
 // LargeStruct bigger than 1024 bytes
 struct LargeStruct* ls = malloc(sizeof(struct LargeStruct));
-DHAT_HISTOGRAM_MEMORY_INIT(ls);
+DHAT_HISTOGRAM_MEMORY(ls);
 ]]></programlisting>
 
 <para>The memory that can be profiled in this way with user requests
-has a further upper limit of 25kbytes.</para>
+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++ <computeroutput>new</computeroutput> with user-defined constructors.</para>
 
 <para>Access counts can be useful for identifying data alignment holes or other
 layout inefficiencies.</para>

diff --git a/dhat/tests/user_histo1.cpp b/dhat/tests/user_histo1.cpp
index f5c8e69bbcaea90378b2aa8784a792734c2ccab2..191fde2c5ae2f8c4d0c2268de2db9b4fec600e93 100644 (file)
--- a/dhat/tests/user_histo1.cpp
+++ b/dhat/tests/user_histo1.cpp
@@ -7,7 +7,7 @@
 int main()
 {
    std::vector<uint8_t> 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());
 }