Always inc/decref in sync(CommandBuffers/Queues)

Also remove the decref in unregister cmdbuf/queue.

Also make it not a pointer.

Also don't inc/dec the m_stream member along with vkencoder inc/decrefs
there is supposed to be only one m_stream inc when encoder is created
and then a dec ref when encoder is deleted...

Change-Id: I3948cf94ee7d399725b0ef1edd01cdf9d78fe777

2 files changed, 58 insertions, 81 deletions

diff --git a/system/vulkan_enc/ResourceTracker.cpp b/system/vulkan_enc/ResourceTracker.cpp
index ecf5b1f9..b2bad648 100644
--- a/system/vulkan_enc/ResourceTracker.cpp
+++ b/system/vulkan_enc/ResourceTracker.cpp
@@ -275,12 +275,12 @@ public:
     };
 
     struct VkCommandBuffer_Info {
-        VkEncoder** lastUsedEncoderPtr = nullptr;
+        VkEncoder* lastUsedEncoder = nullptr;
         uint32_t sequenceNumber = 0;
     };
 
     struct VkQueue_Info {
-        VkEncoder** lastUsedEncoderPtr = nullptr;
+        VkEncoder* lastUsedEncoder = nullptr;
         uint32_t sequenceNumber = 0;
     };
 
@@ -398,15 +398,7 @@ public:
         auto it = info_VkCommandBuffer.find(commandBuffer);
         if (it == info_VkCommandBuffer.end()) return;
         auto& info = it->second;
-        auto lastUsedEncoder =
-            info.lastUsedEncoderPtr ?
-            *(info.lastUsedEncoderPtr) : nullptr;
-
-        if (lastUsedEncoder) {
-            lastUsedEncoder->decRef();
-            info.lastUsedEncoderPtr = nullptr;
-        }
-
+        info.lastUsedEncoder = nullptr;
         info_VkCommandBuffer.erase(commandBuffer);
     }
 
@@ -416,15 +408,8 @@ public:
         auto it = info_VkQueue.find(queue);
         if (it == info_VkQueue.end()) return;
         auto& info = it->second;
-        auto lastUsedEncoder =
-            info.lastUsedEncoderPtr ?
-            *(info.lastUsedEncoderPtr) : nullptr;
-
-        if (lastUsedEncoder) {
-            lastUsedEncoder->decRef();
-            info.lastUsedEncoderPtr = nullptr;
-        }
-
+        auto lastUsedEncoder = info.lastUsedEncoder;
+        info.lastUsedEncoder = nullptr;
         info_VkQueue.erase(queue);
     }
 
@@ -4828,42 +4813,39 @@ public:
 
         auto& info = it->second;
 
-        if (!info.lastUsedEncoderPtr) {
-            info.lastUsedEncoderPtr = new VkEncoder*;
-            *(info.lastUsedEncoderPtr) = currentEncoder;
-            currentEncoder->incRef();
-        }
-
-        auto lastUsedEncoderPtr = info.lastUsedEncoderPtr;
-
-        auto lastEncoder = *(lastUsedEncoderPtr);
-
-        // We always make lastUsedEncoderPtr track
-        // the current encoder, even if the last encoder
-        // is null. And also, increment the current encoder,
-        // and decrement refcount for the last one.
-        // (Decrement only after we are done)
-        *(lastUsedEncoderPtr) = currentEncoder;
-        if (lastEncoder != currentEncoder) {
-            currentEncoder->incRef();
-        }
+        currentEncoder->incRef();
 
+        auto lastEncoder = info.lastUsedEncoder;
+        info.lastUsedEncoder = currentEncoder;
         if (!lastEncoder) return 0;
-        if (lastEncoder == currentEncoder) return 0;
-
         auto oldSeq = info.sequenceNumber;
+        bool lookupAgain = false;
 
-        lock.unlock();
+        if (lastEncoder != currentEncoder) {
+            info.sequenceNumber += 2;
+            lookupAgain = true;
+            lock.unlock();
 
-        lastEncoder->vkCommandBufferHostSyncGOOGLE(commandBuffer, false, oldSeq + 1);
-        lastEncoder->flush();
+            lastEncoder->vkCommandBufferHostSyncGOOGLE(commandBuffer, false, oldSeq + 1);
+            lastEncoder->flush();
 
-        currentEncoder->vkCommandBufferHostSyncGOOGLE(commandBuffer, true, oldSeq + 2);
+            currentEncoder->vkCommandBufferHostSyncGOOGLE(commandBuffer, true, oldSeq + 2);
 
-        lock.lock();
-        if (lastEncoder->decRef()) { *lastUsedEncoderPtr = nullptr; }
+            lock.lock();
+        }
 
-        return 1;
+        if (lastEncoder->decRef()) {
+            if (lookupAgain) {
+                auto it2 = info_VkCommandBuffer.find(commandBuffer);
+                if (it2 == info_VkCommandBuffer.end()) return 0;
+                auto& info2 = it2->second;
+                info2.lastUsedEncoder = nullptr;
+            } else {
+                info.lastUsedEncoder = nullptr;
+            }
+        }
+
+        return 0;
     }
 
     uint32_t syncEncodersForQueue(VkQueue queue, VkEncoder* currentEncoder) {
@@ -4878,44 +4860,38 @@ public:
 
         auto& info = it->second;
 
-        if (!info.lastUsedEncoderPtr) {
-            info.lastUsedEncoderPtr = new VkEncoder*;
-            *(info.lastUsedEncoderPtr) = currentEncoder;
-            currentEncoder->incRef();
-        }
-
-        auto lastUsedEncoderPtr = info.lastUsedEncoderPtr;
-
-        auto lastEncoder = *(lastUsedEncoderPtr);
-
-        // We always make lastUsedEncoderPtr track
-        // the current encoder, even if the last encoder
-        // is null.
-        *(lastUsedEncoderPtr) = currentEncoder;
-
-        if (lastEncoder != currentEncoder) {
-            currentEncoder->incRef();
-        }
+        currentEncoder->incRef();
 
+        auto lastEncoder = info.lastUsedEncoder;
+        info.lastUsedEncoder = currentEncoder;
         if (!lastEncoder) return 0;
-        if (lastEncoder == currentEncoder) return 0;
-
         auto oldSeq = info.sequenceNumber;
+        bool lookupAgain = false;
 
-        info.sequenceNumber += 2;
-
-        lock.unlock();
+        if (lastEncoder != currentEncoder) {
+            info.sequenceNumber += 2;
+            lookupAgain = true;
+            lock.unlock();
 
-        // at this point the seqno for the old thread is determined
+            lastEncoder->vkQueueHostSyncGOOGLE(queue, false, oldSeq + 1);
+            lastEncoder->flush();
+            currentEncoder->vkQueueHostSyncGOOGLE(queue, true, oldSeq + 2);
 
-        lastEncoder->vkQueueHostSyncGOOGLE(queue, false, oldSeq + 1);
-        lastEncoder->flush();
-        currentEncoder->vkQueueHostSyncGOOGLE(queue, true, oldSeq + 2);
+            lock.lock();
+        }
 
-        lock.lock();
-        if (lastEncoder->decRef()) { *lastUsedEncoderPtr = nullptr; }
+        if (lastEncoder->decRef()) {
+            if (lookupAgain) {
+                auto it2 = info_VkQueue.find(queue);
+                if (it2 == info_VkQueue.end()) return 0;
+                auto& info2 = it2->second;
+                info2.lastUsedEncoder = nullptr;
+            } else {
+                info.lastUsedEncoder = nullptr;
+            }
+        }
 
-        return 1;
+        return 0;
     }
 
     VkResult on_vkBeginCommandBuffer(
diff --git a/system/vulkan_enc/VkEncoder.cpp b/system/vulkan_enc/VkEncoder.cpp
index 0d5e4801..111bf400 100644
--- a/system/vulkan_enc/VkEncoder.cpp
+++ b/system/vulkan_enc/VkEncoder.cpp
@@ -62,6 +62,7 @@ using android::base::Pool;
 class VkEncoder::Impl {
 public:
     Impl(IOStream* stream) : m_stream(stream), m_logEncodes(false) {
+        m_stream.incStreamRef();
         const char* emuVkLogEncodesPropName = "qemu.vk.log";
         char encodeProp[PROPERTY_VALUE_MAX];
         if (property_get(emuVkLogEncodesPropName, encodeProp, nullptr) > 0) {
@@ -69,7 +70,9 @@ public:
         }
     }
 
-    ~Impl() { }
+    ~Impl() {
+        m_stream.decStreamRef();
+    }
 
     VulkanCountingStream* countingStream() { return &m_countingStream; }
     VulkanStreamGuest* stream() { return &m_stream; }
@@ -114,11 +117,9 @@ public:
 
     void incRef() {
         __atomic_add_fetch(&m_refCount, 1, __ATOMIC_SEQ_CST);
-        m_stream.incStreamRef();
     }
 
     bool decRef() {
-        m_stream.decStreamRef();
         if (0 == __atomic_sub_fetch(&m_refCount, 1, __ATOMIC_SEQ_CST)) {
             return true;
         }