better documentation for 64-bit frozen. (#758)

Author: lemire

CMakeLists.txt

@@ -3,6 +3,7 @@ project(RoaringBitmap
   DESCRIPTION "Roaring bitmaps in C (and C++)"
   LANGUAGES CXX C
 )
+set(CMAKE_INTERPROCEDURAL_OPTIMIZATION TRUE)
 set (CMAKE_C_STANDARD 11)
 set (CMAKE_CXX_STANDARD 11)
 

cpp/roaring/roaring64map.hh

@@ -1250,9 +1250,11 @@ class Roaring64Map {
     }
 
     /**
-     * For advanced users only.
+     * For advanced users only. This function is unsafe. You must ensure that
+     * the provided buffer is 32-byte aligned.
      */
     static const Roaring64Map frozenView(const char *buf) {
+        // We do not check that buf is 32-byte aligned. Caller is responsible.
         // size of bitmap buffer and key
         const size_t metadata_size = sizeof(size_t) + sizeof(uint32_t);
 
@@ -1311,16 +1313,24 @@ class Roaring64Map {
         return result;
     }
 
+    /**
+     * For advanced users only. Offered on a best-effort basis.
+     * If you use this function in production, you are responsible for
+     * testing it on your target platforms. This function is unsafe.
+     */
     // As with serialized 64-bit bitmaps, 64-bit frozen bitmaps are serialized
     // by concatenating one or more Roaring::write output buffers with the
-    // preceeding map key. Unlike standard bitmap serialization, frozen bitmaps
-    // must be 32-byte aligned and requires a buffer length to parse. As a
-    // result, each concatenated output of Roaring::writeFrozen is preceeded by
-    // padding, the buffer size (size_t), and the map key (uint32_t). The
-    // padding is used to ensure 32-byte alignment, but since it is followed by
-    // the buffer size and map key, it actually pads to `(x - sizeof(size_t) +
-    // sizeof(uint32_t)) mod 32` to leave room for the metadata.
+    // preceeding map key. Like the 32-bit bitmaps, it expects that the provided
+    // buffer is 32-byte aligned. The caller is responsible to check the
+    // alignment. Unlike standard bitmap serialization, frozen bitmaps must be
+    // 32-byte aligned and requires a buffer length to parse. As a result, each
+    // concatenated output of Roaring::writeFrozen is preceeded by padding, the
+    // buffer size (size_t), and the map key (uint32_t). The padding is used to
+    // ensure 32-byte alignment, but since it is followed by the buffer size and
+    // map key, it actually pads to `(x - sizeof(size_t) + sizeof(uint32_t)) mod
+    // 32` to leave room for the metadata.
     void writeFrozen(char *buf) const {
+        // We do not check that buf is 32-byte aligned. Caller is responsible.
         // size of bitmap buffer and key
         const size_t metadata_size = sizeof(size_t) + sizeof(uint32_t);
 
@@ -1349,6 +1359,9 @@ class Roaring64Map {
         }
     }
 
+    /**
+     * For advanced users only. This function is unsafe.
+     */
     size_t getFrozenSizeInBytes() const {
         // size of bitmap size and map key
         const size_t metadata_size = sizeof(size_t) + sizeof(uint32_t);

microbenchmarks/synthetic_bench.cpp

@@ -417,9 +417,9 @@ static void cppFrozenSerialize(benchmark::State& state) {
         r.add(val);
     }
     size_t size = r.getFrozenSizeInBytes();
-    // TODO: there seems to be a bug in writeFrozen that causes writes beyond
-    // getFrozenSizeInBytes()
-    std::vector<char> buf(size * 2);
+    // writeFrozen requires a 32-byte aligned buffer. It is inconvenient to
+    // enforce that here, so we just overallocate.
+    std::vector<char> buf(size * 2 + 32);
     for (auto _ : state) {
         r.writeFrozen(buf.data());
         benchmark::DoNotOptimize(buf);
@@ -504,9 +504,9 @@ static void cppFrozenDeserialize(benchmark::State& state) {
         r1.add(val);
     }
     size_t size = r1.getFrozenSizeInBytes();
-    // TODO: there seems to be a bug in writeFrozen that causes writes beyond
-    // getFrozenSizeInBytes()
-    std::vector<char> buf(size * 2);
+    // writeFrozen requires a 32-byte aligned buffer. It is inconvenient to
+    // enforce that here, so we just overallocate.
+    std::vector<char> buf(size * 2 + 32);
     r1.writeFrozen(buf.data());
     for (auto _ : state) {
         auto r2 = Roaring64Map::frozenView(buf.data());