feat: unify constructors

Author: gengdy1545

cpp/pixels-retina/include/RGVisibility.h

@@ -26,8 +26,8 @@
 template<size_t CAPACITY>
 class RGVisibility : public pixels::RetinaBase<RGVisibility<CAPACITY>> {
 public:
-    explicit RGVisibility(uint64_t rgRecordNum);
-    explicit RGVisibility(uint64_t rgRecordNum, uint64_t timestamp, const std::vector<uint64_t>& initialBitmap);
+    explicit RGVisibility(uint64_t rgRecordNum, uint64_t timestamp = 0,
+                          const std::vector<uint64_t>* initialBitmap = nullptr);
     ~RGVisibility() override;
 
     void deleteRGRecord(uint32_t rowId, uint64_t timestamp);

cpp/pixels-retina/include/RGVisibilityJni.h

@@ -65,13 +65,13 @@ struct VersionedData : public pixels::RetinaBase<VersionedData<CAPACITY>> {
     uint64_t baseTimestamp;
     DeleteIndexBlock* head; // Delete chain head, part of the version
 
-    VersionedData() : baseTimestamp(0), head(nullptr) {
-        std::memset(baseBitmap, 0, sizeof(baseBitmap));
-    }
-
-    VersionedData(uint64_t ts, const uint64_t* bitmap, DeleteIndexBlock* h)
+    // timestamp defaults to 0; bitmap defaults to all-zeros.
+    explicit VersionedData(uint64_t ts = 0, const uint64_t* bitmap = nullptr, DeleteIndexBlock* h = nullptr)
         : baseTimestamp(ts), head(h) {
-        std::memcpy(baseBitmap, bitmap, NUM_WORDS * sizeof(uint64_t));
+        if (bitmap)
+            std::memcpy(baseBitmap, bitmap, NUM_WORDS * sizeof(uint64_t));
+        else
+            std::memset(baseBitmap, 0, sizeof(baseBitmap));
     }
 };
 
@@ -92,8 +92,8 @@ template<size_t CAPACITY>
 class TileVisibility : public pixels::RetinaBase<TileVisibility<CAPACITY>> {
     static constexpr size_t NUM_WORDS = BITMAP_WORDS(CAPACITY);
  public:
-    TileVisibility();
-    TileVisibility(uint64_t ts, const uint64_t* bitmap);
+    // timestamp defaults to 0; bitmap defaults to all-zeros.
+    explicit TileVisibility(uint64_t timestamp = 0, const uint64_t* bitmap = nullptr);
     ~TileVisibility() override;
     void deleteTileRecord(uint16_t rowId, uint64_t ts);
     void getTileVisibilityBitmap(uint64_t ts, uint64_t* outBitmap) const;

cpp/pixels-retina/include/TileVisibility.h

@@ -22,35 +22,23 @@
 #include <cstring>
 #include <thread>
 
+// Validates before allocation: any throw leaves tileVisibilities as nullptr,
+// so the incomplete constructor does not invoke the destructor (no memory leak).
 template<size_t CAPACITY>
-RGVisibility<CAPACITY>::RGVisibility(uint64_t rgRecordNum)
-    : tileCount((rgRecordNum + VISIBILITY_RECORD_CAPACITY - 1) / VISIBILITY_RECORD_CAPACITY) {
-    size_t allocSize = tileCount * sizeof(TileVisibility<CAPACITY>);
-    void* rawMemory = operator new[](allocSize);
-    tileVisibilities = static_cast<TileVisibility<CAPACITY>*>(rawMemory);
-    for (uint64_t i = 0; i < tileCount; ++i) {
-        new (&tileVisibilities[i]) TileVisibility<CAPACITY>();
-    }
-}
-
-template<size_t CAPACITY>
-RGVisibility<CAPACITY>::RGVisibility(uint64_t rgRecordNum, uint64_t timestamp, const std::vector<uint64_t>& initialBitmap)
-    : tileCount((rgRecordNum + VISIBILITY_RECORD_CAPACITY - 1) / VISIBILITY_RECORD_CAPACITY) {
-    size_t allocSize = tileCount * sizeof(TileVisibility<CAPACITY>);
-    void* rawMemory = operator new[](allocSize);
+RGVisibility<CAPACITY>::RGVisibility(uint64_t rgRecordNum, uint64_t timestamp,
+                                      const std::vector<uint64_t>* initialBitmap)
+    : tileCount((rgRecordNum + VISIBILITY_RECORD_CAPACITY - 1) / VISIBILITY_RECORD_CAPACITY),
+      tileVisibilities(nullptr) {
+    if (initialBitmap && initialBitmap->size() < tileCount * BITMAP_SIZE_PER_TILE_VISIBILITY)
+        throw std::invalid_argument("Initial bitmap size is too small for the given record number.");
 
-    if (initialBitmap.size() < tileCount * BITMAP_SIZE_PER_TILE_VISIBILITY) {
-        operator delete[](rawMemory);
-        throw std::runtime_error("Initial bitmap size is too small for the given record number.");
-    }
+    tileVisibilities = static_cast<TileVisibility<CAPACITY>*>(
+        operator new[](tileCount * sizeof(TileVisibility<CAPACITY>)));
 
-    tileVisibilities = static_cast<TileVisibility<CAPACITY>*>(rawMemory);
-    for (uint64_t i = 0; i < tileCount; ++i) {
-        // Each tile takes 4 uint64_t
-        const uint64_t* tileBitmap = &initialBitmap[i * BITMAP_SIZE_PER_TILE_VISIBILITY];
-        // We use timestamp 0 for restored checkpoints to serve as the base state
-        new (&tileVisibilities[i]) TileVisibility<CAPACITY>(timestamp, tileBitmap);
-    }
+    for (uint64_t i = 0; i < tileCount; ++i)
+        new (&tileVisibilities[i]) TileVisibility<CAPACITY>(
+            timestamp,
+            initialBitmap ? initialBitmap->data() + i * BITMAP_SIZE_PER_TILE_VISIBILITY : nullptr);
 }
 
 template<size_t CAPACITY>

cpp/pixels-retina/lib/RGVisibility.cpp

@@ -26,40 +26,28 @@
 /*
  * Class:     io_pixelsdb_pixels_retina_RGVisibility
  * Method:    createNativeObject
- * Signature: (J)J
- */
-JNIEXPORT jlong JNICALL Java_io_pixelsdb_pixels_retina_RGVisibility_createNativeObject
-  (JNIEnv* env, jobject, jlong rgRecordNum) {
-    try {
-        auto* rgVisibility = new RGVisibilityInstance(rgRecordNum);
-        return reinterpret_cast<jlong>(rgVisibility);
-    } catch (const std::exception& e) {
-        env->ThrowNew(env->FindClass("java/lang/RuntimeException"), e.what());
-        return 0;
-    }
-}
-
-/*
- * Class:     io_pixelsdb_pixels_retina_RGVisibility
- * Method:    createNativeObjectInitialized
  * Signature: (JJ[J)J
+ *
+ * Converts the Java bitmap array to a native vector when present, then
+ * forwards to a single RGVisibility constructor call.
  */
-JNIEXPORT jlong JNICALL Java_io_pixelsdb_pixels_retina_RGVisibility_createNativeObjectInitialized
+JNIEXPORT jlong JNICALL Java_io_pixelsdb_pixels_retina_RGVisibility_createNativeObject
   (JNIEnv* env, jobject, jlong rgRecordNum, jlong timestamp, jlongArray bitmap) {
     try {
-        jsize len = env->GetArrayLength(bitmap);
-        jlong *body = env->GetLongArrayElements(bitmap, nullptr);
-
         std::vector<uint64_t> bitmapData;
-        bitmapData.reserve(len);
-        for (int i = 0; i < len; i++) {
-            bitmapData.push_back((uint64_t)body[i]);
+        const std::vector<uint64_t>* bitmapPtr = nullptr;
+        if (bitmap != nullptr) {
+            jsize len = env->GetArrayLength(bitmap);
+            jlong* body = env->GetLongArrayElements(bitmap, nullptr);
+            bitmapData.assign(reinterpret_cast<uint64_t*>(body),
+                              reinterpret_cast<uint64_t*>(body) + len);
+            env->ReleaseLongArrayElements(bitmap, body, JNI_ABORT);
+            bitmapPtr = &bitmapData;
         }
-
-        env->ReleaseLongArrayElements(bitmap, body, JNI_ABORT);
-
-        RGVisibilityInstance *rgVisibility = new RGVisibilityInstance(rgRecordNum, timestamp, bitmapData);
-        return reinterpret_cast<jlong>(rgVisibility);
+        return reinterpret_cast<jlong>(new RGVisibilityInstance(
+            static_cast<uint64_t>(rgRecordNum),
+            static_cast<uint64_t>(timestamp),
+            bitmapPtr));
     } catch (const std::exception& e) {
         env->ThrowNew(env->FindClass("java/lang/RuntimeException"), e.what());
         return 0;
@@ -195,6 +183,5 @@ JNIEXPORT jlong JNICALL Java_io_pixelsdb_pixels_retina_RGVisibility_getRetinaTra
  */
 JNIEXPORT jlong JNICALL Java_io_pixelsdb_pixels_retina_RGVisibility_getRetinaObjectCount
   (JNIEnv *env, jclass clazz) {
-    // Read the atomic object counter from RetinaBase namespace
     return static_cast<jlong>(pixels::g_retina_object_count.load(std::memory_order_relaxed));
 }

cpp/pixels-retina/lib/RGVisibilityJni.cpp

@@ -27,20 +27,9 @@
 #include <immintrin.h>
 
 template<size_t CAPACITY>
-TileVisibility<CAPACITY>::TileVisibility() {
-    VersionedData<CAPACITY>* initialVersion = new VersionedData<CAPACITY>();
-    currentVersion.store(initialVersion, std::memory_order_release);
-    tail.store(nullptr, std::memory_order_release);
-    tailUsed.store(0, std::memory_order_release);
-}
-
-template<size_t CAPACITY>
-TileVisibility<CAPACITY>::TileVisibility(uint64_t ts, const uint64_t* bitmap) {
-    VersionedData<CAPACITY>* initialVersion = new VersionedData<CAPACITY>(ts, bitmap, nullptr);
-    currentVersion.store(initialVersion, std::memory_order_release);
-    tail.store(nullptr, std::memory_order_release);
-    tailUsed.store(0, std::memory_order_release);
-}
+TileVisibility<CAPACITY>::TileVisibility(uint64_t timestamp, const uint64_t* bitmap)
+    : currentVersion(new VersionedData<CAPACITY>(timestamp, bitmap)),
+      tail(nullptr), tailUsed(0) {}
 
 template<size_t CAPACITY>
 TileVisibility<CAPACITY>::~TileVisibility() {
@@ -392,4 +381,4 @@ void TileVisibility<CAPACITY>::reclaimRetiredVersions() {
 }
 
 // Explicit Instantiations (Add the sizes you need here)
-template class TileVisibility<RETINA_CAPACITY>;
+template class TileVisibility<RETINA_CAPACITY>;

cpp/pixels-retina/lib/TileVisibility.cpp

@@ -68,19 +68,32 @@ public class RGVisibility implements AutoCloseable
     public RGVisibility(long rgRecordNum)
     {
         this.recordNum = rgRecordNum;
-        this.nativeHandle.set(createNativeObject(rgRecordNum));
+        this.nativeHandle.set(createNativeObject(rgRecordNum, 0L, null));
     }
 
-    public RGVisibility(long rgRecordNum, long timestamp, long[] initialBitmap)
+    public RGVisibility(long rgRecordNum, long timestamp)
     {
+        if (timestamp <= 0)
+        {
+            throw new IllegalArgumentException("timestamp must be positive when provided");
+        }
         this.recordNum = rgRecordNum;
-        if (initialBitmap == null)
+        this.nativeHandle.set(createNativeObject(rgRecordNum, timestamp, null));
+    }
+
+    public RGVisibility(long rgRecordNum, long timestamp, long[] initialBitmap)
+    {
+        if (timestamp <= 0)
         {
-            this.nativeHandle.set(createNativeObject(rgRecordNum));
-        } else
+            throw new IllegalArgumentException("timestamp must be positive when provided");
+        }
+        
+        if (initialBitmap == null)
         {
-            this.nativeHandle.set(createNativeObjectInitialized(rgRecordNum, timestamp, initialBitmap));
+            throw new IllegalArgumentException("initial bitmap must not be null");
         }
+        this.recordNum = rgRecordNum;
+        this.nativeHandle.set(createNativeObject(rgRecordNum, timestamp, initialBitmap));
     }
 
     public long getRecordNum()
@@ -99,8 +112,7 @@ public void close()
     }
 
     // native methods
-    private native long createNativeObject(long rgRecordNum);
-    private native long createNativeObjectInitialized(long rgRecordNum, long timestamp, long[] bitmap);
+    private native long createNativeObject(long rgRecordNum, long timestamp, long[] bitmap);
     private native void destroyNativeObject(long nativeHandle);
     private native void deleteRecord(int rgRowOffset, long timestamp, long nativeHandle);
     private native long[] getVisibilityBitmap(long timestamp, long nativeHandle);