Bug fixes and performance improvements to PacketManager and HistoryManager. Switched to lazy refreshes when a render occurs rather than doing it in the processing path. Fixed crash in StreamBrowserDialog during startup. Fixes #835.

Author: azonenberg

.gitignore

@@ -5,3 +5,4 @@ analysis-build
 asan-build
 release-nodebug-build
 debug-noopt-build
+release-asan-build

src/ngscopeclient/HistoryManager.cpp

@@ -319,7 +319,7 @@ void HistoryManager::AddHistory(
 				LogTrace("Removing un-pinned waveform at t=%s (now have %zu points of %d allowed)\n",
 					point->m_time.PrettyPrint().c_str(), m_history.size(), m_maxDepth);
 				m_session.RemoveMarkers(point->m_time);
-				m_session.RemovePackets(point->m_time, false);
+				m_session.RemovePackets(point->m_time);
 				m_history.erase(it);
 				deletedSomething = true;
 				break;

src/ngscopeclient/PacketManager.cpp

@@ -44,6 +44,7 @@ using namespace std;
 PacketManager::PacketManager(PacketDecoder* pd, Session& session)
 	: m_session(session)
 	, m_filter(pd)
+	, m_refreshPending(false)
 {
 
 }
@@ -72,6 +73,7 @@ void PacketManager::RefreshRows()
 	lock_guard<recursive_mutex> lock(m_mutex);
 
 	//Clear all existing row state
+	m_refreshPending = false;
 	m_rows.clear();
 
 	//Make a list of waveform timestamps and make sure we display them in order
@@ -185,7 +187,7 @@ void PacketManager::Update()
 	m_cachekey = key;
 
 	//Remove any old history we might have had from this timestamp
-	RemoveHistoryFrom(time, false);
+	RemoveHistoryFrom(time);
 
 	//Copy the new packets and detach them so the filter doesn't delete them.
 	//Do the merging now
@@ -259,8 +261,7 @@ void PacketManager::FilterPackets()
 		m_filteredChildPackets = m_childPackets;
 
 		//but still refresh the set of rows being displayed
-		RefreshRows();
-
+		m_refreshPending = true;
 		return;
 	}
 
@@ -301,18 +302,15 @@ void PacketManager::FilterPackets()
 		}
 	}
 
-	//Refresh the set of rows being displayed
-	RefreshRows();
+	m_refreshPending = true;
 }
 
 /**
 	@brief Removes all history from the specified timestamp
 
 	@param timestamp		Time to remove the history from
-	@param refreshAfter		True if we should refresh the list of displayed rows, false to not refresh
-							(should only be set false in Update())
  */
-void PacketManager::RemoveHistoryFrom(TimePoint timestamp, bool refreshAfter)
+void PacketManager::RemoveHistoryFrom(TimePoint timestamp)
 {
 	lock_guard<recursive_mutex> lock(m_mutex);
 
@@ -329,8 +327,7 @@ void PacketManager::RemoveHistoryFrom(TimePoint timestamp, bool refreshAfter)
 	m_filteredPackets.erase(timestamp);
 
 	//update the list of displayed rows so we don't have anything left pointing to stale packets
-	if(refreshAfter)
-		RefreshRows();
+	m_refreshPending = true;
 }
 
 void PacketManager::RemoveChildHistoryFrom(Packet* pack)

src/ngscopeclient/PacketManager.h

@@ -156,7 +156,7 @@ class PacketManager
 	virtual ~PacketManager();
 
 	void Update();
-	void RemoveHistoryFrom(TimePoint timestamp, bool refreshAfter = true);
+	void RemoveHistoryFrom(TimePoint timestamp);
 
 	std::recursive_mutex& GetMutex()
 	{ return m_mutex; }
@@ -191,10 +191,26 @@ class PacketManager
 	{ m_lastChildOpen[pack] = open; }
 
 	std::vector<RowData>& GetRows()
-	{ return m_rows; }
+	{
+		RefreshIfPending();
+		return m_rows;
+	}
 
 	void OnMarkerChanged();
 
+	/**
+		@brief Refresh the list of pending packets
+	 */
+	void RefreshIfPending()
+	{
+		std::lock_guard<std::recursive_mutex> lock(m_mutex);
+		if(m_refreshPending)
+		{
+			LogTrace("Refreshing rows for %s due to pending changes\n", m_filter->GetDisplayName().c_str());
+			RefreshRows();
+		}
+	}
+
 protected:
 	void RemoveChildHistoryFrom(Packet* pack);
 
@@ -233,6 +249,9 @@ class PacketManager
 
 	///@brief Map of packets to child-open flags from last frame
 	std::map<Packet*, bool> m_lastChildOpen;
+
+	///@brief True if we have a refresh pending before we can render (i.e. pending deletion or similar)
+	bool m_refreshPending;
 };
 
 #endif

src/ngscopeclient/Session.cpp

@@ -140,7 +140,7 @@ void Session::ClearBackgroundThreads()
 	}
 
 	//Clear our trigger state
-	//Important to signal the WaveformProcessingThread so it doesn't block waiting on response that's not going to come
+	//Important to signal the WaveformThread so it doesn't block waiting on response that's not going to come
 	g_waveformReadyEvent.Clear();
 	g_rerenderDoneEvent.Clear();
 	g_waveformProcessedEvent.Signal();
@@ -153,6 +153,9 @@ void Session::ClearBackgroundThreads()
 
 	//Clear shutdown flag in case we're reusing the session object
 	m_shuttingDown = false;
+
+	//Clear the WaveformThread signal if it's not already cleared
+	g_waveformProcessedEvent.Clear();
 }
 
 void Session::FlushConfigCache()
@@ -3473,10 +3476,10 @@ shared_ptr<PacketManager> Session::AddPacketFilter(PacketDecoder* filter)
 /**
 	@brief Deletes packets from our packet managers for a waveform timestamp
  */
-void Session::RemovePackets(TimePoint t, bool immediateRefresh)
+void Session::RemovePackets(TimePoint t)
 {
 	for(auto it : m_packetmgrs)
-		it.second->RemoveHistoryFrom(t, immediateRefresh);
+		it.second->RemoveHistoryFrom(t);
 }
 
 ////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////

src/ngscopeclient/Session.h

@@ -535,7 +535,7 @@ class Session
 	void RemoveMarkers(TimePoint t)
 	{ m_markers.erase(t); }
 
-	void RemovePackets(TimePoint t, bool immediateRefresh = true);
+	void RemovePackets(TimePoint t);
 
 	std::set<FlowGraphNode*> GetAllGraphNodes();
 

src/ngscopeclient/StreamBrowserDialog.h

@@ -55,7 +55,11 @@ class StreamBrowserTimebaseInfo
 	int m_rate;
 
 	uint64_t GetRate()
-	{ return m_rates[m_rate]; }
+	{
+		if(m_rates.empty())
+			return 0;
+		return m_rates[m_rate];
+	}
 
 	//Memory depth
 	std::vector<uint64_t> m_depths;