cpp/openttd-patchpack/source Changeset - r26537:76b6e14a8c30

Changeset - r26537:76b6e14a8c30

Parent rev.

Child rev.

[Not reviewed]

master

0 2 0

Jonathan G Rennison - 2 years ago 2022-11-09 23:12:30
j.g.rennison@gmail.com

Fix: Data race with mixer thread performance measurements

2 files changed with 44 insertions and 0 deletions:

src/framerate_gui.cpp

src/window.cpp

0 comments (0 inline, 0 general)

src/framerate_gui.cpp

➞

Show inline comments

@@ @@ -25,8 +25,16 @@ @@
 #include "game/game_instance.hpp"
 #include "widgets/framerate_widget.h"
 #include <atomic>
 #include <mutex>
 #include <vector>
 #include "safeguards.h"
 static std::mutex _sound_perf_lock;
 static std::atomic<bool> _sound_perf_pending;
 static std::vector<TimingMeasurement> _sound_perf_measurements;
 /**
  * Private declarations for performance measurement implementation
@@ @@ -251,6 +259,20 @@ PerformanceMeasurer::~PerformanceMeasure @@
 			return;
+		}
+	}
 	if (this->elem == PFE_SOUND) {
 		/* PFE_SOUND measurements are made from the mixer thread.
 		 * _pf_data cannot be concurrently accessed from the mixer thread
 		 * and the main thread, so store the measurement results in a
 		 * mutex-protected queue which is drained by the main thread.
 		 * See: ProcessPendingPerformanceMeasurements() */
 		TimingMeasurement end = GetPerformanceTimer();
 		std::lock_guard lk(_sound_perf_lock);
 		if (_sound_perf_measurements.size() >= NUM_FRAMERATE_POINTS * 2) return;
 		_sound_perf_measurements.push_back(this->start_time);
 		_sound_perf_measurements.push_back(end);
 		_sound_perf_pending.store(true, std::memory_order_release);
 		return;
+	}
 	_pf_data[this->elem].Add(this->start_time, GetPerformanceTimer());
+}
@@ @@ -1079,3 +1101,22 @@ void ConPrintFramerate() @@
 		IConsolePrint(CC_ERROR, "No performance measurements have been taken yet.");
+	}
+}
 /**
  * This drains the PFE_SOUND measurement data queue into _pf_data.
  * PFE_SOUND measurements are made by the mixer thread and so cannot be stored
  * into _pf_data directly, because this would not be thread safe and would violate
  * the invariants of the FPS and frame graph windows.
  * @see PerformanceMeasurement::~PerformanceMeasurement()
  */
 void ProcessPendingPerformanceMeasurements()
+{
 	if (_sound_perf_pending.load(std::memory_order_acquire)) {
 		std::lock_guard lk(_sound_perf_lock);
 		for (size_t i = 0; i < _sound_perf_measurements.size(); i += 2) {
 			_pf_data[PFE_SOUND].Add(_sound_perf_measurements[i], _sound_perf_measurements[i + 1]);
+		}
 		_sound_perf_measurements.clear();
 		_sound_perf_pending.store(false, std::memory_order_relaxed);
+	}
+}

src/window.cpp

➞

Show inline comments

@@ @@ -3067,6 +3067,9 @@ void UpdateWindows() @@
 	PerformanceMeasurer framerate(PFE_DRAWING);
 	PerformanceAccumulator::Reset(PFE_DRAWWORLD);
 	extern void ProcessPendingPerformanceMeasurements();
 	ProcessPendingPerformanceMeasurements();
 	CallWindowRealtimeTickEvent(delta_ms);
 	static GUITimer network_message_timer = GUITimer(1);

0 comments (0 inline, 0 general)