Fix two issues in the Vulkan version of GPA

Make sure results are ready before trying to query them
GPA_ContinueSampleOnCommandList failure

Author: chesik-amd

Src/GPUPerfAPI-Common/GPAPass.cpp

@@ -84,6 +84,11 @@ GPASample* GPAPass::GetSampleById(ClientSampleId sampleId) const
 {
     std::lock_guard<std::mutex> lock(m_samplesMapMutex);
 
+    return GetSampleById_NotThreadSafe(sampleId);
+}
+
+GPASample* GPAPass::GetSampleById_NotThreadSafe(ClientSampleId sampleId) const
+{
     GPASample* pRetVal = nullptr;
 
     if (m_samplesMap.find(sampleId) != m_samplesMap.end())
@@ -154,7 +159,7 @@ bool GPAPass::ContinueSample(ClientSampleId srcSampleId, IGPACommandList* pPrima
     // We will mark the parent sample as to be continued by the client
 
     bool success = false;
-    GPASample* pParentSample = GetSampleById(srcSampleId);
+    GPASample* pParentSample = GetSampleById_NotThreadSafe(srcSampleId);
 
     if (nullptr != pParentSample)
     {
@@ -179,14 +184,19 @@ bool GPAPass::ContinueSample(ClientSampleId srcSampleId, IGPACommandList* pPrima
 
                 if (nullptr != pNewSample)
                 {
-                    // TODO: Can these two be merged into the same entrypoint?
-                    // ie, if we link a continuing sample, isn't it by default continued by client?
-                    // is there a way that WE (GPA) would continue it on our own?
-                    // Mark the parent sample as getting continuing by client
-                    pParentSample->SetAsContinuedByClient();
-                    // Link the sample to the parent sample
-                    pParentSample->LinkContinuingSample(pNewSample);
-                    success = true;
+                    if (!pPrimaryGpaCmdList->BeginSample(srcSampleId, pNewSample))
+                    {
+                        GPA_LogError("Unable to begin continued sample in pass.");
+                        delete pNewSample;
+                        pNewSample = nullptr;
+                    }
+                    else
+                    {
+                        pParentSample->SetAsContinuedByClient();
+                        // Link the sample to the parent sample
+                        pParentSample->LinkContinuingSample(pNewSample);
+                        success = true;
+                    }
                 }
             }
             else

Src/GPUPerfAPI-Common/GPAPass.h

@@ -248,6 +248,12 @@ class GPAPass
     /// \return True if the sample has been opened; False otherwise
     bool DoesSampleExist_NotThreadSafe(ClientSampleId clientSampleId) const;
 
+    /// Returns the sample by its client sample Id
+    /// Does NOT lock the mutex, expects the calling method to do that.
+    /// \param[in] sampleId Id of the sample
+    /// \return GPA Sample object
+    GPASample* GetSampleById_NotThreadSafe(ClientSampleId sampleId) const;
+
     /// Create an API-specific GPASample of the supplied GpaSampleType.
     /// \param[in] pCmdList The commandList on which this sample is taking place.
     /// \param[in] sampleType Indicates whether the created sample should support Software or Hardware counters.

Src/GPUPerfAPI-Common/GPASample.cpp

@@ -212,7 +212,6 @@ GPA_THREAD_SAFE_FUNCTION bool GPASample::SetAsContinuedByClient()
 {
     std::lock_guard<std::mutex> lockSample(m_sampleMutex);
     bool success = false;
-    m_gpaSampleState = GPASampleState::PENDING_RESULTS;
 
     if (!m_isClosedByClient)
     {
@@ -259,6 +258,8 @@ IGPACommandList* GPASample::GetCmdList() const
 
 bool GPASample::LinkContinuingSample(GPASample* pContinuingSample)
 {
+    std::lock_guard<std::recursive_mutex> lock(m_continueSampleMutex);
+
     if (nullptr == pContinuingSample)
     {
         return false;

Src/GPUPerfAPI-Common/GPASample.h

@@ -225,20 +225,21 @@ class GPA_NOT_THREAD_SAFE_OBJECT GPASample
     /// Release allocated counters
     virtual void ReleaseCounters() = 0;
 
-    GPAPass*         m_pPass;               ///< GPA Pass Object
-    IGPACommandList* m_pGpaCmdList;         ///< Pointer to the command list object
-    GpaSampleType    m_gpaSampleType;       ///< type of the GPA sample
-    ClientSampleId   m_clientSampleId;      ///< Client-assigned sample Id
-    DriverSampleId   m_driverSampleId;      ///< Driver created sample id
-    GPASampleState   m_gpaSampleState;      ///< The state of this sample
-    GPASampleResult* m_pSampleResult;       ///< memory for sample Results
-    GPASample*       m_pContinuingSample;   ///< Pointer to linked/continuing GpaSample
-    std::mutex       m_sampleMutex;         ///< mutex for the GPA sample object
-    bool             m_isSecondary;         ///< flag indicating a sample is a secondary sample; i.e. it has been created on a bundle or secondary command buffer
-    bool             m_isOpened;            ///< flag indicating a sample is opened
-    bool             m_isClosedByClient;    ///< flag indicating a sample is closed by the command list on which it is created
-    bool             m_isContinuedByClient; ///< flag indicating a sampe has been continued on another command list
-    bool             m_isCopiedSample;      ///< flag indicating that sample has been copied to primary command list
+    GPAPass*             m_pPass;               ///< GPA Pass Object
+    IGPACommandList*     m_pGpaCmdList;         ///< Pointer to the command list object
+    GpaSampleType        m_gpaSampleType;       ///< type of the GPA sample
+    ClientSampleId       m_clientSampleId;      ///< Client-assigned sample Id
+    DriverSampleId       m_driverSampleId;      ///< Driver created sample id
+    GPASampleState       m_gpaSampleState;      ///< The state of this sample
+    GPASampleResult*     m_pSampleResult;       ///< memory for sample Results
+    GPASample*           m_pContinuingSample;   ///< Pointer to linked/continuing GpaSample
+    std::recursive_mutex m_continueSampleMutex; ///< mutex for the GPA sample object
+    std::mutex           m_sampleMutex;         ///< mutex for the GPA sample object
+    bool                 m_isSecondary;         ///< flag indicating a sample is a secondary sample; i.e. it has been created on a bundle or secondary command buffer
+    bool                 m_isOpened;            ///< flag indicating a sample is opened
+    bool                 m_isClosedByClient;    ///< flag indicating a sample is closed by the command list on which it is created
+    bool                 m_isContinuedByClient; ///< flag indicating a sampe has been continued on another command list
+    bool                 m_isCopiedSample;      ///< flag indicating that sample has been copied to primary command list
 };
 
 #endif // _GPA_SAMPLE_H_

Src/GPUPerfAPI-Common/GPASession.cpp

@@ -378,7 +378,7 @@ bool GPASession::UpdateResults()
 
         if (!areAllPassesComplete)
         {
-            GPA_LogError("Pass is not complete.");
+            GPA_LogDebugMessage("Pass is not complete.");
         }
     }
 

Src/GPUPerfAPIVk/VkGPACommandList.cpp

@@ -175,6 +175,25 @@ bool VkGPACommandList::CloseLastSampleRequest()
     return true;
 }
 
+bool VkGPACommandList::IsResultReady() const
+{
+    bool isResultReady = false;
+
+    VkGPAContext* pVkGPAContext = dynamic_cast<VkGPAContext*>(GetParentSession()->GetParentContext());
+
+    if (nullptr == pVkGPAContext)
+    {
+       GPA_LogError("Invalid profiling session encountered when checking for available results.");
+    }
+    else
+    {
+        VkResult isReady = _vkGetGpaSessionStatusAMD(pVkGPAContext->GetVkDevice(), m_gpaExtSessionAMD);
+        isResultReady = VK_SUCCESS == isReady;
+    }
+
+    return isResultReady;
+}
+
 VkCommandBuffer VkGPACommandList::GetVkCommandBuffer() const
 {
     return m_vkCmdBuffer;

Src/GPUPerfAPIVk/VkGPACommandList.h

@@ -41,6 +41,9 @@ class VkGPACommandList : public GPACommandList
     /// Destructor
     ~VkGPACommandList();
 
+    /// \copydoc IGPACommandList::IsResultReady()
+    bool IsResultReady() const override;
+
     /// \copydoc IGPAInterfaceTrait::GetAPIType()
     GPA_API_Type GetAPIType() const override;
 

Src/GPUPerfAPIVk/VkGPASession.cpp

@@ -26,7 +26,7 @@ GPA_Status VkGPASession::ContinueSampleOnCommandList(gpa_uint32 srcSampleId, GPA
 {
     bool succeed = false;
 
-    if (primaryCommandListId->Object()->GetAPIType() == GPA_API_DIRECTX_12 &&
+    if (primaryCommandListId->Object()->GetAPIType() == GetAPIType() &&
         primaryCommandListId->ObjectType() == GPAObjectType::GPA_OBJECT_TYPE_COMMAND_LIST)
     {
         VkGPACommandList* pVkGpaCmdList = reinterpret_cast<VkGPACommandList*>(primaryCommandListId->Object());