You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficserver.apache.org by am...@apache.org on 2019/02/13 14:45:12 UTC

[trafficserver] branch master updated: HdrHeap: refresh for C++17.

This is an automated email from the ASF dual-hosted git repository.

amc pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/trafficserver.git


The following commit(s) were added to refs/heads/master by this push:
     new 8213ca0  HdrHeap: refresh for C++17.
8213ca0 is described below

commit 8213ca0293f5b47b0291c0eb3ae083afe1457004
Author: Alan M. Carroll <am...@apache.org>
AuthorDate: Thu Feb 7 18:00:18 2019 -0600

    HdrHeap: refresh for C++17.
---
 doc/developer-guide/core-architecture/heap.en.rst | 13 +++-
 iocore/cache/Cache.cc                             | 12 ++--
 proxy/hdrs/HTTP.cc                                |  2 +-
 proxy/hdrs/HdrHeap.cc                             | 80 +++++++++++------------
 proxy/hdrs/HdrHeap.h                              | 56 ++++++++--------
 proxy/hdrs/HdrTSOnly.cc                           | 25 ++++---
 proxy/hdrs/unit_tests/test_HdrUtils.cc            |  2 +-
 src/traffic_cache_tool/CacheScan.cc               |  5 +-
 src/traffic_server/CoreUtils.cc                   | 12 +---
 9 files changed, 103 insertions(+), 104 deletions(-)

diff --git a/doc/developer-guide/core-architecture/heap.en.rst b/doc/developer-guide/core-architecture/heap.en.rst
index f05fbbb..e03b5ad 100644
--- a/doc/developer-guide/core-architecture/heap.en.rst
+++ b/doc/developer-guide/core-architecture/heap.en.rst
@@ -159,10 +159,17 @@ collection operation on the writeable string heap in the header heap by calling
 
 *  An external string heap is being added and all current read only string heap slots are used.
 
+The mechanism is simple in design - the size of the live string data in the current string heaps is
+calculated and a new heap is allocated sufficient to contain all existing strings, with additional
+space for new string data. Each heap object is required to provide a :code:`strings_length` method
+which returns the size of the live string data for that object (recursively as needed). The strings
+are copied to the new string heap, all of the previous string heaps are discarded, and the new heap
+becomes the writable string heap for the header heap.
+
 Each heap object is responsible for providing a :code:`move_strings` method which copies its strings
-to a new string heap. This is a source of pointer invalidation for other parts of the core and the
-plugin API. For the latter, insulating from such string movement is the point of the
-:c:type:`TSMLoc` type.
+to a new string heap, passed as an argument. This is a source of pointer invalidation for other
+parts of the core and the plugin API. For the latter, insulating from such string movement is the
+point of the :c:type:`TSMLoc` type.
 
 String Allocation
 -----------------
diff --git a/iocore/cache/Cache.cc b/iocore/cache/Cache.cc
index 28bace5..4933727 100644
--- a/iocore/cache/Cache.cc
+++ b/iocore/cache/Cache.cc
@@ -3289,18 +3289,18 @@ CacheProcessor::find_by_path(const char *path, int len)
 
 namespace cache_bc
 {
-static size_t const HTTP_ALT_MARSHAL_SIZE = ROUND(sizeof(HTTPCacheAlt), HDR_PTR_SIZE); // current size.
+static size_t const HTTP_ALT_MARSHAL_SIZE = HdrHeapMarshalBlocks{ts::round_up(sizeof(HTTPCacheAlt))}; // current size.
 size_t
 HTTPInfo_v21::marshalled_length(void *data)
 {
-  size_t zret           = ROUND(sizeof(HTTPCacheAlt_v21), HDR_PTR_SIZE);
+  size_t zret           = HdrHeapMarshalBlocks{ts::round_up(sizeof(HTTPCacheAlt_v21))};
   HTTPCacheAlt_v21 *alt = static_cast<HTTPCacheAlt_v21 *>(data);
   HdrHeap *hdr;
 
   hdr = reinterpret_cast<HdrHeap *>(reinterpret_cast<char *>(alt) + reinterpret_cast<uintptr_t>(alt->m_request_hdr.m_heap));
-  zret += ROUND(hdr->unmarshal_size(), HDR_PTR_SIZE);
+  zret += HdrHeapMarshalBlocks{ts::round_up(hdr->unmarshal_size())};
   hdr = reinterpret_cast<HdrHeap *>(reinterpret_cast<char *>(alt) + reinterpret_cast<uintptr_t>(alt->m_response_hdr.m_heap));
-  zret += ROUND(hdr->unmarshal_size(), HDR_PTR_SIZE);
+  zret += HdrHeapMarshalBlocks{ts::round_up(hdr->unmarshal_size())};
   return zret;
 }
 
@@ -3363,7 +3363,7 @@ HTTPInfo_v21::copy_and_upgrade_unmarshalled_to_v23(char *&dst, char *&src, size_
   s_hdr =
     reinterpret_cast<HdrHeap_v23 *>(reinterpret_cast<char *>(s_alt) + reinterpret_cast<uintptr_t>(s_alt->m_request_hdr.m_heap));
   d_hdr    = reinterpret_cast<HdrHeap_v23 *>(dst);
-  hdr_size = ROUND(s_hdr->unmarshal_size(), HDR_PTR_SIZE);
+  hdr_size = HdrHeapMarshalBlocks{ts::round_up(s_hdr->unmarshal_size())};
   if (hdr_size > length) {
     return false;
   }
@@ -3375,7 +3375,7 @@ HTTPInfo_v21::copy_and_upgrade_unmarshalled_to_v23(char *&dst, char *&src, size_
   s_hdr =
     reinterpret_cast<HdrHeap_v23 *>(reinterpret_cast<char *>(s_alt) + reinterpret_cast<uintptr_t>(s_alt->m_response_hdr.m_heap));
   d_hdr    = reinterpret_cast<HdrHeap_v23 *>(dst);
-  hdr_size = ROUND(s_hdr->unmarshal_size(), HDR_PTR_SIZE);
+  hdr_size = HdrHeapMarshalBlocks{ts::round_up(s_hdr->unmarshal_size())};
   if (hdr_size > length) {
     return false;
   }
diff --git a/proxy/hdrs/HTTP.cc b/proxy/hdrs/HTTP.cc
index 4704bfd..41bf41b 100644
--- a/proxy/hdrs/HTTP.cc
+++ b/proxy/hdrs/HTTP.cc
@@ -1950,7 +1950,7 @@ HTTPCacheAlt::copy_frag_offsets_from(HTTPCacheAlt *src)
   }
 }
 
-const int HTTP_ALT_MARSHAL_SIZE = ROUND(sizeof(HTTPCacheAlt), HDR_PTR_SIZE);
+const int HTTP_ALT_MARSHAL_SIZE = HdrHeapMarshalBlocks{ts::round_up(sizeof(HTTPCacheAlt))};
 
 void
 HTTPInfo::create()
diff --git a/proxy/hdrs/HdrHeap.cc b/proxy/hdrs/HdrHeap.cc
index 243d6ce..cde0e9f 100644
--- a/proxy/hdrs/HdrHeap.cc
+++ b/proxy/hdrs/HdrHeap.cc
@@ -37,11 +37,11 @@
 #include "HTTP.h"
 #include "I_EventSystem.h"
 
-#define MAX_LOST_STR_SPACE 1024
+constexpr size_t MAX_LOST_STR_SPACE = 1024;
 
-Allocator hdrHeapAllocator("hdrHeap", HDR_HEAP_DEFAULT_SIZE);
+Allocator hdrHeapAllocator("hdrHeap", HdrHeap::DEFAULT_SIZE);
 
-Allocator strHeapAllocator("hdrStrHeap", HDR_STR_HEAP_DEFAULT_SIZE);
+Allocator strHeapAllocator("hdrStrHeap", HdrStrHeap::DEFAULT_SIZE);
 
 /*-------------------------------------------------------------------------
   -------------------------------------------------------------------------*/
@@ -114,8 +114,8 @@ HdrHeap *
 new_HdrHeap(int size)
 {
   HdrHeap *h;
-  if (size <= HDR_HEAP_DEFAULT_SIZE) {
-    size = HDR_HEAP_DEFAULT_SIZE;
+  if (size <= HdrHeap::DEFAULT_SIZE) {
+    size = HdrHeap::DEFAULT_SIZE;
     h    = (HdrHeap *)(THREAD_ALLOC(hdrHeapAllocator, this_ethread()));
   } else {
     h = (HdrHeap *)ats_malloc(size);
@@ -138,12 +138,12 @@ new_HdrStrHeap(int requested_size)
   int alloc_size = requested_size + sizeof(HdrStrHeap);
 
   HdrStrHeap *sh;
-  if (alloc_size <= HDR_STR_HEAP_DEFAULT_SIZE) {
-    alloc_size = HDR_STR_HEAP_DEFAULT_SIZE;
+  if (alloc_size <= HdrStrHeap::DEFAULT_SIZE) {
+    alloc_size = HdrStrHeap::DEFAULT_SIZE;
     sh         = (HdrStrHeap *)(THREAD_ALLOC(strHeapAllocator, this_ethread()));
   } else {
-    alloc_size = ROUND(alloc_size, HDR_STR_HEAP_DEFAULT_SIZE * 2);
-    sh         = (HdrStrHeap *)ats_malloc(alloc_size);
+    alloc_size = ts::round_up<HdrStrHeap::DEFAULT_SIZE * 2>(alloc_size);
+    sh         = static_cast<HdrStrHeap *>(ats_malloc(alloc_size));
   }
 
   //    Debug("hdrs", "Allocated string heap in size %d", alloc_size);
@@ -152,8 +152,8 @@ new_HdrStrHeap(int requested_size)
   sh = new (sh) HdrStrHeap();
 
   sh->m_heap_size  = alloc_size;
-  sh->m_free_size  = alloc_size - STR_HEAP_HDR_SIZE;
-  sh->m_free_start = ((char *)sh) + STR_HEAP_HDR_SIZE;
+  sh->m_free_size  = alloc_size - sizeof(HdrStrHeap);
+  sh->m_free_start = reinterpret_cast<char *>(sh + 1);
 
   ink_assert(sh->refcount() == 0);
 
@@ -174,7 +174,7 @@ HdrHeap::destroy()
     i.m_ref_count_ptr = nullptr;
   }
 
-  if (m_size == HDR_HEAP_DEFAULT_SIZE) {
+  if (m_size == HdrHeap::DEFAULT_SIZE) {
     THREAD_FREE(this, hdrHeapAllocator, this_thread());
   } else {
     ats_free(this);
@@ -189,9 +189,9 @@ HdrHeap::allocate_obj(int nbytes, int type)
 
   ink_assert(m_writeable);
 
-  nbytes = ROUND(nbytes, HDR_PTR_SIZE);
+  nbytes = HdrHeapMarshalBlocks{ts::round_up(nbytes)};
 
-  if (nbytes > (int)HDR_MAX_ALLOC_SIZE) {
+  if (nbytes > static_cast<int>(HDR_MAX_ALLOC_SIZE)) {
     ink_assert(!"alloc too big");
     return nullptr;
   }
@@ -253,7 +253,7 @@ RETRY:
   // First check to see if we have a read/write
   //   string heap
   if (!m_read_write_heap) {
-    int next_size     = (last_size * 2) - STR_HEAP_HDR_SIZE;
+    int next_size     = (last_size * 2) - sizeof(HdrStrHeap);
     next_size         = next_size > nbytes ? next_size : nbytes;
     m_read_write_heap = new_HdrStrHeap(next_size);
   }
@@ -562,7 +562,7 @@ HdrHeap::marshal_length()
     }
   }
 
-  len = ROUND(len, HDR_PTR_SIZE);
+  len = HdrHeapMarshalBlocks(ts::round_up(len));
   return len;
 }
 
@@ -624,7 +624,6 @@ HdrHeap::marshal(char *buf, int len)
   // Variables used later on.  Sunpro doesn't like
   //   bypassing initializations with gotos
   int used;
-  int i;
 
   HdrHeap *unmarshal_hdr = this;
 
@@ -660,7 +659,7 @@ HdrHeap::marshal(char *buf, int len)
   // Now that we've got the pointer blocks marshaled
   //  we can fill in the header on marshalled block
   marshal_hdr->m_free_start = nullptr;
-  marshal_hdr->m_data_start = (char *)HDR_HEAP_HDR_SIZE; // offset
+  marshal_hdr->m_data_start = reinterpret_cast<char *>(HDR_HEAP_HDR_SIZE.value()); // offset
   marshal_hdr->m_magic      = HDR_BUF_MAGIC_MARSHALED;
   marshal_hdr->m_writeable  = false;
   marshal_hdr->m_size       = ptr_heap_size + HDR_HEAP_HDR_SIZE;
@@ -673,7 +672,7 @@ HdrHeap::marshal(char *buf, int len)
   marshal_hdr->m_ronly_heap[0].m_heap_start = (char *)(intptr_t)marshal_hdr->m_size; // offset
   marshal_hdr->m_ronly_heap[0].m_ref_count_ptr.detach();
 
-  for (int i = 1; i < HDR_BUF_RONLY_HEAPS; i++) {
+  for (unsigned i = 1; i < HDR_BUF_RONLY_HEAPS; ++i) {
     marshal_hdr->m_ronly_heap[i].m_heap_start = nullptr;
   }
 
@@ -708,7 +707,7 @@ HdrHeap::marshal(char *buf, int len)
     str_heaps++;
   }
 
-  for (i = 0; i < HDR_BUF_RONLY_HEAPS; i++) {
+  for (unsigned i = 0; i < HDR_BUF_RONLY_HEAPS; ++i) {
     if (m_ronly_heap[i].m_heap_start != nullptr) {
       if (m_ronly_heap[i].m_heap_len > len) {
         goto Failed;
@@ -785,7 +784,7 @@ HdrHeap::marshal(char *buf, int len)
 
   // Add up the total bytes used
   used = ptr_heap_size + str_size + HDR_HEAP_HDR_SIZE;
-  used = ROUND(used, HDR_PTR_SIZE);
+  used = HdrHeapMarshalBlocks(ts::round_up(used));
 
 #ifdef HDR_HEAP_CHECKSUMS
   {
@@ -956,14 +955,14 @@ HdrHeap::unmarshal(int buf_length, int obj_type, HdrHeapObjImpl **found_obj, Ref
 
   m_magic = HDR_BUF_MAGIC_ALIVE;
 
-  unmarshal_size = ROUND(unmarshal_size, HDR_PTR_SIZE);
+  unmarshal_size = HdrHeapMarshalBlocks(ts::round_up(unmarshal_size));
   return unmarshal_size;
 }
 
 inline bool
-HdrHeap::attach_str_heap(char *h_start, int h_len, RefCountObj *h_ref_obj, int *index)
+HdrHeap::attach_str_heap(char const *h_start, int h_len, RefCountObj *h_ref_obj, int *index)
 {
-  if (*index >= HDR_BUF_RONLY_HEAPS) {
+  if (static_cast<unsigned>(*index) >= HDR_BUF_RONLY_HEAPS) {
     return false;
   }
 
@@ -1005,14 +1004,13 @@ HdrHeap::inherit_string_heaps(const HdrHeap *inherit_from)
     return;
   }
 
-  int index;
   int first_free       = HDR_BUF_RONLY_HEAPS; // default is out of array bounds
   int free_slots       = 0;
   int inherit_str_size = 0;
   ink_assert(m_writeable);
 
   // Find the number of free heap slots & the first open index
-  for (index = 0; index < HDR_BUF_RONLY_HEAPS; index++) {
+  for (unsigned index = 0; index < HDR_BUF_RONLY_HEAPS; ++index) {
     if (m_ronly_heap[index].m_heap_start == nullptr) {
       if (first_free == HDR_BUF_RONLY_HEAPS) {
         first_free = index;
@@ -1026,7 +1024,7 @@ HdrHeap::inherit_string_heaps(const HdrHeap *inherit_from)
     free_slots--;
     inherit_str_size = inherit_from->m_read_write_heap->m_heap_size;
   }
-  for (index = 0; index < HDR_BUF_RONLY_HEAPS; index++) {
+  for (unsigned index = 0; index < HDR_BUF_RONLY_HEAPS; ++index) {
     if (inherit_from->m_ronly_heap[index].m_heap_start != nullptr) {
       free_slots--;
       inherit_str_size += inherit_from->m_ronly_heap[index].m_heap_len;
@@ -1054,8 +1052,8 @@ HdrHeap::inherit_string_heaps(const HdrHeap *inherit_from)
     // Copy over read/write string heap if it exists
     if (inherit_from->m_read_write_heap) {
       int str_size =
-        inherit_from->m_read_write_heap->m_heap_size - STR_HEAP_HDR_SIZE - inherit_from->m_read_write_heap->m_free_size;
-      ink_release_assert(attach_str_heap(((char *)inherit_from->m_read_write_heap.get()) + STR_HEAP_HDR_SIZE, str_size,
+        inherit_from->m_read_write_heap->m_heap_size - sizeof(HdrStrHeap) - inherit_from->m_read_write_heap->m_free_size;
+      ink_release_assert(attach_str_heap(reinterpret_cast<char *>(inherit_from->m_read_write_heap.get() + 1), str_size,
                                          inherit_from->m_read_write_heap.get(), &first_free));
     }
     // Copy over read only string heaps
@@ -1115,7 +1113,7 @@ HdrHeap::dump_heap(int len)
 void
 HdrStrHeap::free()
 {
-  if (m_heap_size == HDR_STR_HEAP_DEFAULT_SIZE) {
+  if (m_heap_size == HdrStrHeap::DEFAULT_SIZE) {
     THREAD_FREE(this, strHeapAllocator, this_thread());
   } else {
     ats_free(this);
@@ -1152,8 +1150,8 @@ HdrStrHeap::expand(char *ptr, int old_size, int new_size)
 {
   unsigned int expand_size = new_size - old_size;
 
-  ink_assert(ptr >= ((char *)this) + STR_HEAP_HDR_SIZE);
-  ink_assert(ptr < ((char *)this) + m_heap_size);
+  ink_assert(ptr >= reinterpret_cast<char const *>(this + 1));
+  ink_assert(ptr < reinterpret_cast<char const *>(this) + m_heap_size);
 
   if (ptr + old_size == m_free_start && expand_size <= m_free_size) {
     m_free_start += expand_size;
@@ -1174,9 +1172,9 @@ REGRESSION_TEST(HdrHeap_Coalesce)(RegressionTest *t, int /* atype ATS_UNUSED */,
    * demotion of rw heaps to ronly heaps, and finally the coalesce and evacuate behaviours.
    */
 
-  // The amount of space we will need to overflow the StrHdrHeap is HDR_STR_HEAP_DEFAULT_SIZE - STR_HEAP_HDR_SIZE
-  size_t next_rw_heap_size           = HDR_STR_HEAP_DEFAULT_SIZE;
-  size_t next_required_overflow_size = next_rw_heap_size - STR_HEAP_HDR_SIZE;
+  // The amount of space we will need to overflow the StrHdrHeap is HdrStrHeap::DEFAULT_SIZE - sizeof(HdrStrHeap)
+  size_t next_rw_heap_size           = HdrStrHeap::DEFAULT_SIZE;
+  size_t next_required_overflow_size = next_rw_heap_size - sizeof(HdrStrHeap);
   char buf[next_required_overflow_size];
   for (unsigned int i = 0; i < sizeof(buf); ++i) {
     buf[i] = ('a' + (i % 26));
@@ -1189,15 +1187,15 @@ REGRESSION_TEST(HdrHeap_Coalesce)(RegressionTest *t, int /* atype ATS_UNUSED */,
   tb.check(heap->m_read_write_heap.get() == nullptr, "Checking that we have no rw heap.");
   url_path_set(heap, url, buf, next_required_overflow_size, true);
   tb.check(heap->m_read_write_heap->m_free_size == 0, "Checking that we've completely consumed the rw heap");
-  for (int i = 0; i < HDR_BUF_RONLY_HEAPS; ++i) {
+  for (unsigned i = 0; i < HDR_BUF_RONLY_HEAPS; ++i) {
     tb.check(heap->m_ronly_heap[i].m_heap_start == (char *)nullptr, "Checking ronly_heap[%d] is NULL", i);
   }
 
   // Now we have no ronly heaps in use and a completely full rwheap, so we will test that
   // we demote to ronly heaps HDR_BUF_RONLY_HEAPS times.
-  for (int ronly_heap = 0; ronly_heap < HDR_BUF_RONLY_HEAPS; ++ronly_heap) {
+  for (unsigned ronly_heap = 0; ronly_heap < HDR_BUF_RONLY_HEAPS; ++ronly_heap) {
     next_rw_heap_size           = 2 * heap->m_read_write_heap->m_heap_size;
-    next_required_overflow_size = next_rw_heap_size - STR_HEAP_HDR_SIZE;
+    next_required_overflow_size = next_rw_heap_size - sizeof(HdrStrHeap);
     char buf2[next_required_overflow_size];
     for (unsigned int i = 0; i < sizeof(buf2); ++i) {
       buf2[i] = ('a' + (i % 26));
@@ -1210,13 +1208,13 @@ REGRESSION_TEST(HdrHeap_Coalesce)(RegressionTest *t, int /* atype ATS_UNUSED */,
              (int)next_rw_heap_size);
     tb.check(heap->m_read_write_heap->m_free_size == 0, "Checking that we've completely consumed the rw heap");
     tb.check(heap->m_ronly_heap[ronly_heap].m_heap_start != nullptr, "Checking that we properly demoted the previous rw heap");
-    for (int i = ronly_heap + 1; i < HDR_BUF_RONLY_HEAPS; ++i) {
+    for (unsigned i = ronly_heap + 1; i < HDR_BUF_RONLY_HEAPS; ++i) {
       tb.check(heap->m_ronly_heap[i].m_heap_start == nullptr, "Checking ronly_heap[%d] is NULL", i);
     }
   }
 
   // We will rerun these checks after we introduce a non-copied string to make sure we didn't already coalesce
-  for (int i = 0; i < HDR_BUF_RONLY_HEAPS; ++i) {
+  for (unsigned i = 0; i < HDR_BUF_RONLY_HEAPS; ++i) {
     tb.check(heap->m_ronly_heap[i].m_heap_start != (char *)nullptr, "Pre non-copied string: Checking ronly_heap[%d] is NOT NULL",
              i);
   }
@@ -1234,7 +1232,7 @@ REGRESSION_TEST(HdrHeap_Coalesce)(RegressionTest *t, int /* atype ATS_UNUSED */,
            "Checking that the aliased string shows having proper length");
   tb.check(aliased_str_url->m_ptr_path == buf3, "Checking that the aliased string is correctly pointing at buf");
 
-  for (int i = 0; i < HDR_BUF_RONLY_HEAPS; ++i) {
+  for (unsigned i = 0; i < HDR_BUF_RONLY_HEAPS; ++i) {
     tb.check(heap->m_ronly_heap[i].m_heap_start != (char *)nullptr, "Post non-copied string: Checking ronly_heap[%d] is NOT NULL",
              i);
   }
diff --git a/proxy/hdrs/HdrHeap.h b/proxy/hdrs/HdrHeap.h
index 9737782..6dd7deb 100644
--- a/proxy/hdrs/HdrHeap.h
+++ b/proxy/hdrs/HdrHeap.h
@@ -36,28 +36,21 @@
 #include "tscore/ink_defs.h"
 #include "tscore/ink_assert.h"
 #include "tscore/Arena.h"
+#include "tscore/Scalar.h"
 #include "HdrToken.h"
 
 // Objects in the heap must currently be aligned to 8 byte boundaries,
 // so their (address & HDR_PTR_ALIGNMENT_MASK) == 0
 
-#define HDR_PTR_SIZE (sizeof(uint64_t))
-#define HDR_PTR_ALIGNMENT_MASK ((HDR_PTR_SIZE)-1L)
-
-#define ROUND(x, l) (((x) + ((l)-1L)) & ~((l)-1L))
+static constexpr size_t HDR_PTR_SIZE           = sizeof(uint64_t);
+static constexpr size_t HDR_PTR_ALIGNMENT_MASK = HDR_PTR_SIZE - 1L;
+using HdrHeapMarshalBlocks                     = ts::Scalar<HDR_PTR_SIZE>;
 
 // A many of the operations regarding read-only str
 //  heaps are hand unrolled in the code.  Chaning
 //  this value requires a full pass through HdrBuf.cc
 //  to fix the unrolled operations
-#define HDR_BUF_RONLY_HEAPS 3
-
-#define HDR_HEAP_DEFAULT_SIZE 2048
-#define HDR_STR_HEAP_DEFAULT_SIZE 2048
-
-#define HDR_MAX_ALLOC_SIZE (HDR_HEAP_DEFAULT_SIZE - sizeof(HdrHeap))
-#define HDR_HEAP_HDR_SIZE ROUND(sizeof(HdrHeap), HDR_PTR_SIZE)
-#define STR_HEAP_HDR_SIZE sizeof(HdrStrHeap)
+static constexpr unsigned HDR_BUF_RONLY_HEAPS = 3;
 
 class CoreUtils;
 class IOBufferBlock;
@@ -140,6 +133,8 @@ enum {
 class HdrStrHeap : public RefCountObj
 {
 public:
+  static constexpr int DEFAULT_SIZE = 2048;
+
   void free() override;
 
   char *allocate(int nbytes);
@@ -150,20 +145,22 @@ public:
   char *m_free_start;
   uint32_t m_free_size;
 
-  bool
-  contains(const char *str) const
-  {
-    return (str >= ((const char *)this + STR_HEAP_HDR_SIZE) && str < ((const char *)this + m_heap_size));
-  }
+  bool contains(const char *str) const;
 };
 
+inline bool
+HdrStrHeap::contains(const char *str) const
+{
+  return reinterpret_cast<char const *>(this + 1) <= str && str < reinterpret_cast<char const *>(this) + m_heap_size;
+}
+
 struct StrHeapDesc {
   StrHeapDesc() = default;
 
   Ptr<RefCountObj> m_ref_count_ptr;
-  char *m_heap_start = nullptr;
-  int32_t m_heap_len = 0;
-  bool m_locked      = false;
+  char const *m_heap_start = nullptr;
+  int32_t m_heap_len       = 0;
+  bool m_locked            = false;
 
   bool
   contains(const char *str) const
@@ -177,6 +174,8 @@ class HdrHeap
   friend class CoreUtils;
 
 public:
+  static constexpr int DEFAULT_SIZE = 2048;
+
   void init();
   inkcoreapi void destroy();
 
@@ -265,7 +264,7 @@ public:
   void coalesce_str_heaps(int incoming_size = 0);
   void evacuate_from_str_heaps(HdrStrHeap *new_heap);
   size_t required_space_for_evacuation();
-  bool attach_str_heap(char *h_start, int h_len, RefCountObj *h_ref_obj, int *index);
+  bool attach_str_heap(char const *h_start, int h_len, RefCountObj *h_ref_obj, int *index);
 
   /** Struct to prevent garbage collection on heaps.
       This bumps the reference count to the heap containing the pointer
@@ -302,6 +301,9 @@ public:
   int m_lost_string_space;
 };
 
+static constexpr HdrHeapMarshalBlocks HDR_HEAP_HDR_SIZE{ts::round_up(sizeof(HdrHeap))};
+static constexpr size_t HDR_MAX_ALLOC_SIZE = HdrHeap::DEFAULT_SIZE - HDR_HEAP_HDR_SIZE;
+
 inline void
 HdrHeap::free_string(const char *s, int len)
 {
@@ -318,15 +320,15 @@ HdrHeap::unmarshal_size() const
 
 //
 struct MarshalXlate {
-  char *start;
-  char *end;
-  char *offset;
+  char const *start;
+  char const *end;
+  char const *offset;
   MarshalXlate() : start(nullptr), end(nullptr), offset(nullptr) {}
 };
 
 struct HeapCheck {
-  char *start;
-  char *end;
+  char const *start;
+  char const *end;
 };
 
 // Nasty macro to do string marshalling
@@ -479,6 +481,6 @@ HdrHeapSDKHandle::set(const HdrHeapSDKHandle *from)
 }
 
 HdrStrHeap *new_HdrStrHeap(int requested_size);
-inkcoreapi HdrHeap *new_HdrHeap(int size = HDR_HEAP_DEFAULT_SIZE);
+inkcoreapi HdrHeap *new_HdrHeap(int size = HdrHeap::DEFAULT_SIZE);
 
 void hdr_heap_test();
diff --git a/proxy/hdrs/HdrTSOnly.cc b/proxy/hdrs/HdrTSOnly.cc
index 6841b1e..55e2d93 100644
--- a/proxy/hdrs/HdrTSOnly.cc
+++ b/proxy/hdrs/HdrTSOnly.cc
@@ -170,30 +170,29 @@ HdrHeap::attach_block(IOBufferBlock *b, const char *use_start)
 
 RETRY:
 
-  // It's my contention that since heaps are add to the
-  //   first available slot, one you find an empty slot
-  //   it's not possible that a heap ptr for this block
-  //   exists in a later slot
-  for (int i = 0; i < HDR_BUF_RONLY_HEAPS; i++) {
-    if (m_ronly_heap[i].m_heap_start == nullptr) {
+  // It's my contention that since heaps are add to the first available slot, one you find an empty
+  // slot it's not possible that a heap ptr for this block exists in a later slot
+
+  for (auto &heap : m_ronly_heap) {
+    if (heap.m_heap_start == nullptr) {
       // Add block to heap in this slot
-      m_ronly_heap[i].m_heap_start    = (char *)use_start;
-      m_ronly_heap[i].m_heap_len      = (int)(b->end() - b->start());
-      m_ronly_heap[i].m_ref_count_ptr = b->data.object();
+      heap.m_heap_start    = static_cast<char const *>(use_start);
+      heap.m_heap_len      = static_cast<int>(b->end() - b->start());
+      heap.m_ref_count_ptr = b->data.object();
       //          printf("Attaching block at %X for %d in slot %d\n",
       //                 m_ronly_heap[i].m_heap_start,
       //                 m_ronly_heap[i].m_heap_len,
       //                 i);
-      return i;
-    } else if (m_ronly_heap[i].m_heap_start == b->buf()) {
+      return &heap - m_ronly_heap;
+    } else if (heap.m_heap_start == b->buf()) {
       // This block is already on the heap so just extend
       //   it's range
-      m_ronly_heap[i].m_heap_len = (int)(b->end() - b->buf());
+      heap.m_heap_len = static_cast<int>(b->end() - b->buf());
       //          printf("Extending block at %X to %d in slot %d\n",
       //                 m_ronly_heap[i].m_heap_start,
       //                 m_ronly_heap[i].m_heap_len,
       //                 i);
-      return i;
+      return &heap - m_ronly_heap;
     }
   }
 
diff --git a/proxy/hdrs/unit_tests/test_HdrUtils.cc b/proxy/hdrs/unit_tests/test_HdrUtils.cc
index fc178a2..fe085c9 100644
--- a/proxy/hdrs/unit_tests/test_HdrUtils.cc
+++ b/proxy/hdrs/unit_tests/test_HdrUtils.cc
@@ -47,7 +47,7 @@ TEST_CASE("HdrUtils", "[proxy][hdrutils]")
   static constexpr std::string_view FOUR_TAG{"Four"};
   static constexpr std::string_view FIVE_TAG{"Five"};
 
-  HdrHeap *heap = new_HdrHeap(HDR_HEAP_DEFAULT_SIZE + 64);
+  HdrHeap *heap = new_HdrHeap(HdrHeap::DEFAULT_SIZE + 64);
   MIMEParser parser;
   char const *real_s = text.data();
   char const *real_e = text.data_end();
diff --git a/src/traffic_cache_tool/CacheScan.cc b/src/traffic_cache_tool/CacheScan.cc
index 06be535..e52b256 100644
--- a/src/traffic_cache_tool/CacheScan.cc
+++ b/src/traffic_cache_tool/CacheScan.cc
@@ -29,7 +29,7 @@
 
 // using namespace ct;
 
-const int HTTP_ALT_MARSHAL_SIZE = ROUND(sizeof(HTTPCacheAlt), HDR_PTR_SIZE);
+constexpr HdrHeapMarshalBlocks HTTP_ALT_MARSHAL_SIZE = ts::round_up(sizeof(HTTPCacheAlt));
 
 namespace ct
 {
@@ -248,8 +248,7 @@ CacheScan::unmarshal(HdrHeap *hh, int buf_length, int obj_type, HdrHeapObjImpl *
 
   hh->m_magic = HDR_BUF_MAGIC_ALIVE;
 
-  int unmarshal_length = ROUND(hh->unmarshal_size(), HDR_PTR_SIZE);
-  return unmarshal_length;
+  return HdrHeapMarshalBlocks(ts::round_up(hh->unmarshal_size()));
 }
 
 Errata
diff --git a/src/traffic_server/CoreUtils.cc b/src/traffic_server/CoreUtils.cc
index eac83b0..15ddb38 100644
--- a/src/traffic_server/CoreUtils.cc
+++ b/src/traffic_server/CoreUtils.cc
@@ -469,10 +469,7 @@ CoreUtils::load_http_hdr(HTTPHdr *core_hdr, HTTPHdr *live_hdr)
   intptr_t ptr_heap_size = 0;
   intptr_t str_size      = 0;
   intptr_t str_heaps     = 0;
-  std::vector<struct MarshalXlate> ptr_xlation(2);
-  // MarshalXlate static_table[2];
-  // MarshalXlate* ptr_xlation = static_table;
-  intptr_t used;
+  std::vector<MarshalXlate> ptr_xlation(2);
   intptr_t i;
   intptr_t copy_size;
 
@@ -544,7 +541,7 @@ CoreUtils::load_http_hdr(HTTPHdr *core_hdr, HTTPHdr *live_hdr)
   swizzle_heap->m_ronly_heap[0].m_heap_start          = (char *)(intptr_t)swizzle_heap->m_size; // offset
   swizzle_heap->m_ronly_heap[0].m_ref_count_ptr.m_ptr = nullptr;
 
-  for (int i = 1; i < HDR_BUF_RONLY_HEAPS; i++) {
+  for (unsigned i = 1; i < HDR_BUF_RONLY_HEAPS; ++i) {
     swizzle_heap->m_ronly_heap[i].m_heap_start = nullptr;
   }
 
@@ -650,10 +647,7 @@ CoreUtils::load_http_hdr(HTTPHdr *core_hdr, HTTPHdr *live_hdr)
   }
 
   // Add up the total bytes used
-  used = ptr_heap_size + str_size + HDR_HEAP_HDR_SIZE;
-  used = ROUND(used, HDR_PTR_SIZE);
-
-  return used;
+  return HdrHeapMarshalBlocks{ts::round_up(ptr_heap_size + str_size + HDR_HEAP_HDR_SIZE)};
 
 Failed:
   swizzle_heap->m_magic = HDR_BUF_MAGIC_CORRUPT;