Repository: RetroAchievements/RAIntegration
Pull request #870 (2022-02-18 22:48:56)

fix performance issue using Continuous Filter with Initial Value and large result sets
When using Continuous Filter, the current memory state is merged into the last result set. In order to find where to do the merge, a linear search is performed. However, due to the abstraction of an `auto`, the linear search was duplicating the data, not just scanning it. This resulted in a very noticeable performance impact. (i.e. taking over a minute instead of fractions of a second when processing the 32MB of memory provided by PSP).

Additionally, this sets an upper limit of 10 seconds on the auto-filter timer. To minimize the performance impact of running the continuous search, the filter is applied less frequently if there are many results. The more results, the less frequently it was applied. There was no upper limit, so if 32 million results remained, the auto-filter would only occur once every five minutes.


Patch:
diff --git a/src/RA_Shared.rc b/src/RA_Shared.rc
index cb18360a..db09f876 100644
--- a/src/RA_Shared.rc
+++ b/src/RA_Shared.rc
@@ -272,16 +272,16 @@ BEGIN
 END
 
 IDD_RA_CODENOTES DIALOGEX 0, 0, 287, 230
-STYLE DS_SETFONT | WS_POPUP | WS_CAPTION | WS_SYSMENU | WS_THICKFRAME
+STYLE DS_SETFONT | DS_FIXEDSYS | WS_POPUP | WS_CAPTION | WS_SYSMENU | WS_THICKFRAME
 CAPTION "Code Notes"
 FONT 8, "MS Shell Dlg", 400, 0, 0x0
 BEGIN
     LTEXT           "Search:",IDC_STATIC,6,6,33,11
     EDITTEXT        IDC_RA_FILTER_VALUE,37,4,194,14,ES_AUTOHSCROLL
     PUSHBUTTON      "Filter",IDC_RA_APPLY_FILTER,233,4,50,14
-    LTEXT           "Results:",IDC_STATIC,6,24,33,8
-    LTEXT           "0/0",IDC_RA_RESULT_COUNT,37,24,49,8
-    PUSHBUTTON      "Bookmark Selected",IDC_RA_ADDBOOKMARK,90,21,70,14,BS_MULTILINE
+    LTEXT           "Results:",IDC_STATIC,6,24,36,8
+    LTEXT           "99999/99999",IDC_RA_RESULT_COUNT,37,24,42,8
+    PUSHBUTTON      "Bookmark Selected",IDC_RA_ADDBOOKMARK,162,21,70,14
     PUSHBUTTON      "Reset",IDC_RA_RESET_FILTER,233,21,50,14
     CONTROL         "",IDC_RA_LBX_ADDRESSES,"SysListView32",LVS_REPORT | LVS_SINGLESEL | LVS_ALIGNLEFT | LVS_NOSORTHEADER | WS_CLIPCHILDREN | WS_BORDER,4,37,279,189
 END
@@ -330,7 +330,8 @@ BEGIN
     PUSHBUTTON      "&Cancel",IDCANCEL,136,16,60,14
 END
 
-///////////////////////////////////////////////////////////
+
+/////////////////////////////////////////////////////////////////////////////
 //
 // DESIGNINFO
 //
@@ -528,7 +529,7 @@ BEGIN
         HORZGUIDE, 107
         HORZGUIDE, 123
     END
-    
+
     IDD_RA_NEWASSET, DIALOG
     BEGIN
         LEFTMARGIN, 4
@@ -552,7 +553,7 @@ END
 2 TEXTINCLUDE 
 BEGIN
     "#include ""winresrc.h""\r\n"
-    "#define IDC_STATIC -1"\r\n
+    "#define IDC_STATIC -1\r\n"
     "\0"
 END
 
diff --git a/src/RA_StringUtils.h b/src/RA_StringUtils.h
index 60ffc55b..a7606504 100644
--- a/src/RA_StringUtils.h
+++ b/src/RA_StringUtils.h
@@ -62,7 +62,7 @@ _NODISCARD inline const std::string ToString(_In_ const T& value)
     }
     else
     {
-        static_assert(false, "No ToString implementation for type");
+        static_assert(std::is_arithmetic_v<T>, "No ToString implementation for type");
         return std::string();
     }
 }
@@ -132,7 +132,7 @@ _NODISCARD inline const std::wstring ToWString(_In_ const T& value)
     }
     else
     {
-        static_assert(false, "No ToWString implementation for type");
+        static_assert(std::is_arithmetic_v<T>, "No ToWString implementation for type");
         return std::wstring();
     }
 }
@@ -585,7 +585,7 @@ _NODISCARD inline auto StringPrintf(_In_z_ _Printf_format_string_ const CharT* c
     }
     else
     {
-        static_assert(false, "unsupported character type");
+        static_assert(std::is_same_v<CharT, char>, "unsupported character type");
         return std::string();
     }
 }
diff --git a/src/services/AchievementRuntime.cpp b/src/services/AchievementRuntime.cpp
index 0a9dc706..1e6a7bd8 100644
--- a/src/services/AchievementRuntime.cpp
+++ b/src/services/AchievementRuntime.cpp
@@ -42,16 +42,9 @@ int AchievementRuntime::ActivateAchievement(ra::AchievementID nId, const std::st
     std::lock_guard<std::mutex> pLock(m_pMutex);
     EnsureInitialized();
 
-    if (m_bPaused)
-    {
-        // If processing is paused, just queue the achievement. Once processing is unpaused, if the trigger
-        // is not false, the achievement will be moved to the active list. This ensures achievements don't
-        // trigger immediately upon loading the game due to uninitialized memory.
-        m_vQueuedAchievements.insert_or_assign(nId, sTrigger);
-        rc_runtime_deactivate_achievement(&m_pRuntime, nId);
-        return RC_OK;
-    }
-
+    // When an achievement is activated, it's set to Waiting. The state of the achievement must evaluate
+    // to false for at least one frame before it is promoted to Active. This ensures achievements don't
+    // trigger from uninitialized (or randomly initialized) memory when first starting a game.
     return rc_runtime_activate_achievement(&m_pRuntime, nId, sTrigger.c_str(), nullptr, 0);
 }
 
@@ -203,21 +196,6 @@ std::wstring AchievementRuntime::GetRichPresenceDisplayString() const
     return std::wstring(L"No Rich Presence defined.");
 }
 
-void AchievementRuntime::SetPaused(bool bValue) 
-{ 
-    if (!bValue && m_bPaused && !m_vQueuedAchievements.empty())
-    {
-        m_bPaused = false;
-
-        for (const auto pPair : m_vQueuedAchievements)
-            ActivateAchievement(pPair.first, pPair.second);
-
-        m_vQueuedAchievements.clear();
-    }
-
-    m_bPaused = bValue; 
-}
-
 static std::vector<AchievementRuntime::Change>* g_pChanges;
 
 static void map_event_to_change(const rc_runtime_event_t* pRuntimeEvent)
diff --git a/src/services/AchievementRuntime.hh b/src/services/AchievementRuntime.hh
index c2061648..d90d6648 100644
--- a/src/services/AchievementRuntime.hh
+++ b/src/services/AchievementRuntime.hh
@@ -233,7 +233,7 @@ public:
     /// <summary>
     /// Sets whether achievement processing should be temporarily suspended.
     /// </summary>
-    void SetPaused(bool bValue);
+    void SetPaused(bool bValue) noexcept { m_bPaused = bValue; }
 
     /// <summary>
     /// Resets any active achievements and disables them until their triggers are false.
@@ -262,7 +262,6 @@ private:
 
     void EnsureInitialized() noexcept;
 
-    std::map<unsigned int, std::string> m_vQueuedAchievements;
     int m_nRichPresenceParseResult = RC_OK;
     int m_nRichPresenceErrorLine = 0;
     bool m_bInitialized = false;
diff --git a/src/services/SearchResults.cpp b/src/services/SearchResults.cpp
index e1183679..33f7ad63 100644
--- a/src/services/SearchResults.cpp
+++ b/src/services/SearchResults.cpp
@@ -1601,7 +1601,7 @@ void SearchResults::MergeSearchResults(const SearchResults& srMemory, const Sear
     m_nFilterType = srAddresses.m_nFilterType;
     m_nFilterValue = srAddresses.m_nFilterValue;
 
-    for (const auto pSrcBlock : srAddresses.m_vBlocks)
+    for (const auto& pSrcBlock : srAddresses.m_vBlocks)
     {
         // copy the block from the srAddresses collection, then update the memory from the srMemory collection.
         // this creates a new block with the memory from the srMemory collection and the addresses from the
@@ -1611,7 +1611,7 @@ void SearchResults::MergeSearchResults(const SearchResults& srMemory, const Sear
         ra::ByteAddress nAddress = pNewBlock.GetFirstAddress();
         unsigned char* pWrite = pNewBlock.GetBytes();
 
-        for (const auto pMemBlock : srMemory.m_vBlocks)
+        for (const auto& pMemBlock : srMemory.m_vBlocks)
         {
             if (nAddress >= pMemBlock.GetFirstAddress() && nAddress < pMemBlock.GetFirstAddress() + pMemBlock.GetBytesSize())
             {
diff --git a/src/ui/viewmodels/MemorySearchViewModel.cpp b/src/ui/viewmodels/MemorySearchViewModel.cpp
index 8bbd66aa..980b9305 100644
--- a/src/ui/viewmodels/MemorySearchViewModel.cpp
+++ b/src/ui/viewmodels/MemorySearchViewModel.cpp
@@ -637,13 +637,15 @@ void MemorySearchViewModel::ApplyContinuousFilter()
     // for 10000 results, only filter every 100ms
     // for 50000 results, only filter every 500ms
     // for 100000 results, only filter every second
+    // up to a max of one filter every ten seconds at 1000000 or more results
     const auto nResults = pResult.pResults.MatchingAddressCount();
     if (nResults > 1000)
     {
         const auto tNow = ra::services::ServiceLocator::Get<ra::services::IClock>().UpTime();
         const auto nElapsed = std::chrono::duration_cast<std::chrono::milliseconds>(tNow - m_tLastContinuousFilter);
+        const auto nElapsedMilliseconds = gsl::narrow_cast<size_t>(nElapsed.count());
 
-        if (gsl::narrow_cast<size_t>(nElapsed.count()) < nResults / 100)
+        if (nElapsedMilliseconds < 10000 && nElapsedMilliseconds < nResults / 100)
             return;
 
         m_tLastContinuousFilter = tNow;
diff --git a/src/ui/win32/CodeNotesDialog.cpp b/src/ui/win32/CodeNotesDialog.cpp
index 59060949..48c1344d 100644
--- a/src/ui/win32/CodeNotesDialog.cpp
+++ b/src/ui/win32/CodeNotesDialog.cpp
@@ -103,9 +103,12 @@ CodeNotesDialog::CodeNotesDialog(CodeNotesViewModel& vmCodeNotes)
     using namespace ra::bitwise_ops;
     SetAnchor(IDC_RA_FILTER_VALUE, Anchor::Top | Anchor::Left | Anchor::Right);
     SetAnchor(IDC_RA_APPLY_FILTER, Anchor::Top | Anchor::Right);
-    SetAnchor(IDC_RA_ADDBOOKMARK, Anchor::Top);
+    SetAnchor(IDC_RA_RESULT_COUNT, Anchor::Top | Anchor::Left);
+    SetAnchor(IDC_RA_ADDBOOKMARK, Anchor::Top | Anchor::Right);
     SetAnchor(IDC_RA_RESET_FILTER, Anchor::Top | Anchor::Right);
     SetAnchor(IDC_RA_LBX_ADDRESSES, Anchor::Top | Anchor::Left | Anchor::Bottom | Anchor::Right);
+
+    SetMinimumSize(327, 200);
 }
 
 BOOL CodeNotesDialog::OnInitDialog()


Test patch:
diff --git a/tests/services/AchievementRuntime_Tests.cpp b/tests/services/AchievementRuntime_Tests.cpp
index 2710692c..46f389ec 100644
--- a/tests/services/AchievementRuntime_Tests.cpp
+++ b/tests/services/AchievementRuntime_Tests.cpp
@@ -230,6 +230,51 @@ public:
         AssertChange(vChanges, AchievementRuntime::ChangeType::AchievementTriggered, 6U);
     }
 
+    TEST_METHOD(TestReactivateAchievementPaused)
+    {
+        std::array<unsigned char, 1> memory{ 0x00 };
+
+        AchievementRuntimeHarness runtime;
+        runtime.mockEmulatorContext.MockMemory(memory);
+        const char* pTriggerDefinition = "0xH0000=1";
+        rc_trigger_t* pTrigger;
+
+        std::vector<AchievementRuntime::Change> vChanges;
+        runtime.ActivateAchievement(6U, pTriggerDefinition);
+        pTrigger = runtime.GetAchievementTrigger(6U);
+        Assert::IsNotNull(pTrigger);
+        Assert::AreEqual(static_cast<int>(RC_TRIGGER_STATE_WAITING), static_cast<int>(pTrigger->state));
+
+        runtime.Process(vChanges);
+        AssertChange(vChanges, AchievementRuntime::ChangeType::AchievementActivated, 6U);
+        Assert::AreEqual(static_cast<int>(RC_TRIGGER_STATE_ACTIVE), static_cast<int>(pTrigger->state));
+        vChanges.clear();
+
+        runtime.SetPaused(true);
+        Assert::IsTrue(runtime.IsPaused());
+
+        runtime.DeactivateAchievement(6U);
+        Assert::IsNull(runtime.GetAchievementTrigger(6U));
+
+        // activating a previously active achievement should resurrect it, even if paused
+        runtime.ActivateAchievement(6U, pTriggerDefinition);
+        pTrigger = runtime.GetAchievementTrigger(6U);
+        Assert::IsNotNull(pTrigger);
+        Assert::AreEqual(static_cast<int>(RC_TRIGGER_STATE_WAITING), static_cast<int>(pTrigger->state));
+
+        // achievement should not become active when paused
+        runtime.SetPaused(true);
+        Assert::IsTrue(runtime.IsPaused());
+        runtime.Process(vChanges);
+        Assert::AreEqual({ 0U }, vChanges.size());
+
+        // processing re-enabled, achievement should become active
+        runtime.SetPaused(false);
+        runtime.Process(vChanges);
+        Assert::AreEqual({ 1U }, vChanges.size());
+        AssertChange(vChanges, AchievementRuntime::ChangeType::AchievementActivated, 6U);
+    }
+
     TEST_METHOD(TestMonitorAchievementReset)
     {
         std::array<unsigned char, 5> memory{0x00, 0x12, 0x34, 0xAB, 0x56};