From: Steinar H. Gunderson <sgunderson@bigfoot.com>
Date: Tue, 14 May 2019 16:12:20 +0000 (+0200)
Subject: In ImageInput, make one global update thread instead of one per image.
X-Git-Tag: 1.9.0~44
X-Git-Url: https://git.sesse.net/?p=nageru;a=commitdiff_plain;h=c660c05323237bf118e6844c71703bf96be70d32

In ImageInput, make one global update thread instead of one per image.

Makes for fewer threads (obviously), and also probably makes it easier
to give it an OpenGL context later.
---

diff --git a/nageru/image_input.cpp b/nageru/image_input.cpp
index 0abefe3..12789dc 100644
--- a/nageru/image_input.cpp
+++ b/nageru/image_input.cpp
@@ -79,9 +79,11 @@ shared_ptr<const ImageInput::Image> ImageInput::load_image(const string &filenam
 	}
 
 	all_images[pathname] = load_image_raw(pathname);
-	timespec first_modified = all_images[pathname]->last_modified;
-	update_threads[pathname] =
-		thread(bind(update_thread_func, filename, pathname, first_modified));
+
+	if (!update_thread_started) {
+		update_thread = thread(update_thread_func);
+		update_thread_started = true;
+	}
 
 	return all_images[pathname];
 }
@@ -198,44 +200,58 @@ shared_ptr<const ImageInput::Image> ImageInput::load_image_raw(const string &pat
 	return image;
 }
 
-// Fire up a thread to update the image every second.
+// Fire up a thread to update all images every second.
 // We could do inotify, but this is good enough for now.
-void ImageInput::update_thread_func(const std::string &filename, const std::string &pathname, const timespec &first_modified)
+void ImageInput::update_thread_func()
 {
-	char thread_name[16];
-	snprintf(thread_name, sizeof(thread_name), "Update_%s", filename.c_str());
-	pthread_setname_np(pthread_self(), thread_name);
+	pthread_setname_np(pthread_self(), "Update_Images");
 
-	timespec last_modified = first_modified;
 	struct stat buf;
 	for ( ;; ) {
 		{
 			unique_lock<mutex> lock(threads_should_quit_mu);
 			threads_should_quit_modified.wait_for(lock, chrono::seconds(1), []() { return threads_should_quit; });
 		}
-
 		if (threads_should_quit) {
 			return;
 		}
 
-		if (stat(pathname.c_str(), &buf) != 0) {
-			fprintf(stderr, "%s: Couldn't check for new version, leaving the old in place.\n", pathname.c_str());
-			continue;
-		}
-		if (buf.st_mtim.tv_sec == last_modified.tv_sec &&
-		    buf.st_mtim.tv_nsec == last_modified.tv_nsec) {
-			// Not changed.
-			continue;
+		// Go through all loaded images and see if they need to be updated.
+		// We do one pass first through the array with no I/O, to avoid
+		// blocking the renderer.
+		vector<pair<string, timespec>> images_to_check;
+		{
+			unique_lock<mutex> lock(all_images_lock);
+			for (const auto &pathname_and_image : all_images) {
+				const string pathname = pathname_and_image.first;
+				const timespec last_modified = pathname_and_image.second->last_modified;
+				images_to_check.emplace_back(pathname, last_modified);
+			}
 		}
-		shared_ptr<const Image> image = load_image_raw(pathname);
-		if (image == nullptr) {
-			fprintf(stderr, "Couldn't load image, leaving the old in place.\n");
-			continue;
+
+		for (const auto &pathname_and_timespec : images_to_check) {
+			const string pathname = pathname_and_timespec.first;
+			const timespec last_modified = pathname_and_timespec.second;
+
+			if (stat(pathname.c_str(), &buf) != 0) {
+				fprintf(stderr, "%s: Couldn't check for new version, leaving the old in place.\n", pathname.c_str());
+				continue;
+			}
+			if (buf.st_mtim.tv_sec == last_modified.tv_sec &&
+			    buf.st_mtim.tv_nsec == last_modified.tv_nsec) {
+				// Not changed.
+				continue;
+			}
+
+			shared_ptr<const Image> image = load_image_raw(pathname);
+			if (image == nullptr) {
+				fprintf(stderr, "Couldn't load image, leaving the old in place.\n");
+				continue;
+			}
+
+			unique_lock<mutex> lock(all_images_lock);
+			all_images[pathname] = image;
 		}
-		fprintf(stderr, "Loaded new version of %s from disk.\n", pathname.c_str());
-		lock_guard<mutex> lock(all_images_lock);
-		all_images[pathname] = image;
-		last_modified = image->last_modified;
 	}
 }
 
@@ -246,14 +262,17 @@ void ImageInput::shutdown_updaters()
 		threads_should_quit = true;
 		threads_should_quit_modified.notify_all();
 	}
-	for (auto &it : update_threads) {
-		it.second.join();
+
+	lock_guard<mutex> lock(all_images_lock);
+	if (update_thread_started) {
+		update_thread.join();
 	}
 }
 
 mutex ImageInput::all_images_lock;
 map<string, shared_ptr<const ImageInput::Image>> ImageInput::all_images;
-map<string, thread> ImageInput::update_threads;
+bool ImageInput::update_thread_started = false;
+thread ImageInput::update_thread;
 mutex ImageInput::threads_should_quit_mu;
 bool ImageInput::threads_should_quit = false;
 condition_variable ImageInput::threads_should_quit_modified;
diff --git a/nageru/image_input.h b/nageru/image_input.h
index 6a75df4..ac3c519 100644
--- a/nageru/image_input.h
+++ b/nageru/image_input.h
@@ -36,10 +36,11 @@ private:
 
 	static std::shared_ptr<const Image> load_image(const std::string &filename, const std::string &pathname);
 	static std::shared_ptr<const Image> load_image_raw(const std::string &pathname);
-	static void update_thread_func(const std::string &filename, const std::string &pathname, const timespec &first_modified);
+	static void update_thread_func();
 	static std::mutex all_images_lock;
-	static std::map<std::string, std::shared_ptr<const Image>> all_images;
-	static std::map<std::string, std::thread> update_threads;
+	static std::map<std::string, std::shared_ptr<const Image>> all_images;  // Under all_images_lock.
+	static bool update_thread_started;  // Under all_images_lock.
+	static std::thread update_thread;  // Under all_images_lock.
 
 	static std::mutex threads_should_quit_mu;
 	static bool threads_should_quit;  // Under threads_should_quit_mu.