use 5-byte compressed pointer in FreeList in AllocationClass

Summary:
We set 5-byte compressed pointer as default when managing free allocations in this allocation class. This allows us to avoid templating this class which can lead to cascading templates which would make the codebase complex.

Ran a canary with the change:
https://www.internalfb.com/canvas/document/1516951625859179

Now can we warm roll this change? To know this we will look at the codepath that persists and loads the code back.

1. The list of allocation for an allocation class is stored as a SList. This is where we saveState() of an SList so that we can load it back up. https://www.internalfb.com/code/fbsource/[1cf1875e8c22]/fbcode/cachelib/allocator/datastruct/SList.h?lines=109. It converts a CompressedPointer to a int64_t to store as thrift object.

2. This int64_t that was serialized is read back here: https://www.internalfb.com/code/fbsource/[1cf1875e8c22]/fbcode/cachelib/allocator/datastruct/SList.h?lines=88.

We have shown in the tests that if we persist with a 4-byte compressed pointer which will be stored as int64_t and read it back as a 5-byte compressed pointer, the value will be equivalent meaning both 4-byte and 5-byte compressed pointer will point to the same slab.

Note that an item still uses the 4-byte compressed pointer so during serializing everything will be considered a 4-byte compressed pointer except for the list of allocations in AllocationClass.

Reviewed By: haowu14

Differential Revision: D64539341

fbshipit-source-id: c141ace3655deed62f2c22bfaddbb2917ae2684a

Author: Pranav Bhandari

cachelib/allocator/memory/AllocationClass.cpp

@@ -51,7 +51,7 @@ AllocationClass::AllocationClass(ClassId classId,
       allocationSize_(allocSize),
       slabAlloc_(s),
       freedAllocations_{
-          slabAlloc_.createPtrCompressor<FreeAlloc, CompressedPtr4B>()} {
+          slabAlloc_.createPtrCompressor<FreeAlloc, CompressedPtr5B>()} {
   checkState();
 }
 
@@ -104,7 +104,7 @@ AllocationClass::AllocationClass(
       slabAlloc_(s),
       freedAllocations_(
           *object.freedAllocationsObject(),
-          slabAlloc_.createPtrCompressor<FreeAlloc, CompressedPtr4B>()),
+          slabAlloc_.createPtrCompressor<FreeAlloc, CompressedPtr5B>()),
       canAllocate_(*object.canAllocate()) {
   if (!slabAlloc_.isRestorable()) {
     throw std::logic_error("The allocation class cannot be restored.");
@@ -359,10 +359,10 @@ std::pair<bool, std::vector<void*>> AllocationClass::pruneFreeAllocs(
   // Set the bit to true if the corresponding allocation is freed, false
   // otherwise.
   FreeList freeAllocs{
-      slabAlloc_.createPtrCompressor<FreeAlloc, CompressedPtr4B>()};
+      slabAlloc_.createPtrCompressor<FreeAlloc, CompressedPtr5B>()};
   FreeList notInSlab{
-      slabAlloc_.createPtrCompressor<FreeAlloc, CompressedPtr4B>()};
-  FreeList inSlab{slabAlloc_.createPtrCompressor<FreeAlloc, CompressedPtr4B>()};
+      slabAlloc_.createPtrCompressor<FreeAlloc, CompressedPtr5B>()};
+  FreeList inSlab{slabAlloc_.createPtrCompressor<FreeAlloc, CompressedPtr5B>()};
 
   lock_->lock_combine([&]() {
     // Take the allocation class free list offline

cachelib/allocator/memory/AllocationClass.h

@@ -443,7 +443,7 @@ class AllocationClass {
   // void* is re-interpreted as FreeAlloc* before being stored in the free
   // list.
   struct CACHELIB_PACKED_ATTR FreeAlloc {
-    using CompressedPtrType = facebook::cachelib::CompressedPtr4B;
+    using CompressedPtrType = facebook::cachelib::CompressedPtr5B;
     using PtrCompressor = facebook::cachelib::
         PtrCompressor<FreeAlloc, SlabAllocator, CompressedPtrType>;
     SListHook<FreeAlloc> hook_{};

cachelib/allocator/memory/tests/CompressedPtrTest.cpp

@@ -187,16 +187,48 @@ TEST_F(CompressedPtrTest, Compare) {
       // Both pointer should not be null
       ASSERT_FALSE((ptr.isNull()) || (ptr5b.isNull()));
 
+      // Both pointer should be able to read the serialized value and create
+      // an equivalent compressed pointer.
+      int64_t serialized4b = ptr.saveState();
+      int64_t serialized5b = ptr5b.saveState();
+
       // Both pointer types should return the same raw value when uncompressed
-      ASSERT_EQ(ptr.getRaw(), ptr5b.getRaw());
+      ASSERT_EQ(serialized4b, serialized5b);
 
-      // Both pointers should return the input value when compressed and
-      // uncompressed immediately.
-      ASSERT_EQ(alloc,
+      // We load 4-byte with 5-byte serialized value and vice versa.
+      CompressedPtr4B newPtr4b = CompressedPtr4B(serialized5b);
+      CompressedPtr5B newPtr5b = CompressedPtr5B(serialized4b);
+
+      // Both compressed pointers when uncompressed should return the same
+      // value even 4-byte compressed pointer is loaded with the value
+      // of 5-byte compressed pointer when serialized and vice versa.
+      ASSERT_EQ(
+          m.unCompress<CompressedPtr5B>(newPtr5b, false /* isMultiTiered */),
+          m.unCompress<CompressedPtr4B>(newPtr4b, false /* isMultiTiered */));
+
+      // All the pointer types should return the same value when uncompressed.
+      ASSERT_EQ(m.unCompress<CompressedPtr5B>(ptr5b, false /* isMultiTiered */),
                 m.unCompress<CompressedPtr4B>(ptr, false /* isMultiTiered */));
+
+      ASSERT_EQ(
+          m.unCompress<CompressedPtr5B>(ptr5b, false /* isMultiTiered */),
+          m.unCompress<CompressedPtr5B>(newPtr5b, false /* isMultiTiered */));
+
+      ASSERT_EQ(
+          m.unCompress<CompressedPtr5B>(ptr5b, false /* isMultiTiered */),
+          m.unCompress<CompressedPtr4B>(newPtr4b, false /* isMultiTiered */));
+
       ASSERT_EQ(
-          alloc,
+          m.unCompress<CompressedPtr4B>(ptr, false /* isMultiTiered */),
+          m.unCompress<CompressedPtr4B>(newPtr4b, false /* isMultiTiered */));
+
+      ASSERT_EQ(
+          m.unCompress<CompressedPtr4B>(ptr, false /* isMultiTiered */),
           m.unCompress<CompressedPtr5B>(ptr5b, false /* isMultiTiered */));
+
+      ASSERT_EQ(
+          m.unCompress<CompressedPtr4B>(ptr, false /* isMultiTiered */),
+          m.unCompress<CompressedPtr5B>(newPtr5b, false /* isMultiTiered */));
     }
   }