Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Godot stutters when starting a thread. #67174

Open
KnightNine opened this issue Oct 10, 2022 · 7 comments
Open

Godot stutters when starting a thread. #67174

KnightNine opened this issue Oct 10, 2022 · 7 comments

Comments

@KnightNine
Copy link

Godot version

v3.5.1

System information

Windows 10, i7-8850h

Issue description

Calling start() upon a thread causes the project to stutter intermittently.
release build

It's probably related to this issue #24869

Steps to reproduce

sample code to test:

func thread_func():
	print("test")
var thread = Thread.new()
func _process(delta):
	thread.wait_to_finish()
	thread.start(self,"thread_func")

Minimal reproduction project

No response

@Calinou
Copy link
Member

Calinou commented Oct 10, 2022

Related to #48108 and #58279.

Starting threads on Windows is more expensive than on other platforms. You generally want to start threads before gameplay and keep them idle until you need to run things in them.

@MGilleronFJ
Copy link

MGilleronFJ commented Oct 10, 2022

It should also be noted that Godot Thread class does extra Godot-specific setup when started, notably for GDScript. At least this should be checked to make sure it's really just the cost of Windows threads.
GDScript was also found to be terrible at calling functions in threads (unless release) but that might not be relevant with such simple example code.

@KnightNine
Copy link
Author

KnightNine commented Oct 11, 2022

Related to #48108 and #58279.

Starting threads on Windows is more expensive than on other platforms. You generally want to start threads before gameplay and keep them idle until you need to run things in them.

@Calinou Yeah well I was trying to use one thread to be held and activated with a Semaphore to have a certain functions called in the background via this thread that was instanced at project initialization, But Godot has made it nearly impossible to yield for anything within a thread (which is needed for thread safety reasons) without losing the ability to use the thread after that without restarting it. I've been working on this problem for 2 days now, and it seems like I'll have to mess with the engine to get a usable solution.

I'd super appreciate the help if you know anything about circumventing this issue, even if it involves modifying the Godot Engine.

Edit:
I eventually have managed to solve the issue but not without a lot of trial and error.

@elvisish
Copy link

Related to #48108 and #58279.
Starting threads on Windows is more expensive than on other platforms. You generally want to start threads before gameplay and keep them idle until you need to run things in them.

@Calinou Yeah well I was trying to use one thread to be held and activated with a Semaphore to have a certain functions called in the background via this thread that was instanced at project initialization, But Godot has made it nearly impossible to yield for anything within a thread (which is needed for thread safety reasons) without losing the ability to use the thread after that without restarting it. I've been working on this problem for 2 days now, and it seems like I'll have to mess with the engine to get a usable solution.

I'd super appreciate the help if you know anything about circumventing this issue, even if it involves modifying the Godot Engine.

Edit: I eventually have managed to solve the issue but not without a lot of trial and error.

Was there a fix for this? I'm starting threads and there's stutter on Windows, and using semaphores to keep threads alive is incredibly confusing.

@Calinou
Copy link
Member

Calinou commented Mar 19, 2023

Was there a fix for this?

No, we don't have a definitive way to resolve this.

@KnightNine Could you share the workaround you mentioned?

I eventually have managed to solve the issue but not without a lot of trial and error.

@KnightNine
Copy link
Author

KnightNine commented Mar 19, 2023

@Calinou

here's some code which I isolated for testing in 3.5 that represents a weeks worth of frustration:
Hopefully it will work when translating it to 4.0 but I haven't gotten to it yet.

extends Spatial

var thread = Thread.new()
var semaphore = Semaphore.new()
var thread_active = true
signal thread_frame()
#in the case where the yield() is called after emit_signal("thread_frame"), this is needed to ensure that eventually the yield catches the signal
var yielding_on_thread_process = false


func thread_process():
	
	while thread_active:
		
		if !yielding_on_thread_process:
			print("held")
			semaphore.wait()
			print("released")
		#this fixes the deadlocking and error issues (because i think the thread will call emit_signal() at the exact same time as yield() is called in reattach_to_thread(), which seems to break godot ) (maybe make the delay a bit larger than 1 usec just in case):
		OS.delay_usec(100)
		emit_signal("thread_frame")
		
		

#this function needs to be called like this in order to work properly: 
""" 
var func_state = reattach_to_thread()
if func_state is GDScriptFunctionState:
	yield(func_state,"completed")
"""
#you could call it like this `yield(reattach_to_thread(),"completed")` but it will crash if you are already in the thread

func reattach_to_thread():
	var id = thread.get_id()
	#if not already in thread
	if OS.get_thread_caller_id() != int(id):
		#on semaphore.post(), the thread is "released" to emit a signal
		yielding_on_thread_process = true
		print("posted")
		semaphore.post()
		#this yield merges/hooks the function into the thread (yielding on anything adds the code being run to whatever thread is being yielded on)
		yield(self,"thread_frame")
		print("hooked")
		yielding_on_thread_process = false
	else:
		print("reattach not needed")
	

#HOW TO USE THESE FUNCTIONS:
# Called when the node enters the scene tree for the first time.
func _ready():
	#start the thread process
	thread.start(self,"thread_process")
	
	print("you are currently in the main thread:"+str(OS.get_thread_caller_id()))
	
	
	var func_state = reattach_to_thread()
	if func_state is GDScriptFunctionState:
		yield(func_state,"completed")
	
	print("you are in the thread now:"+str(OS.get_thread_caller_id()))
	
	#return to main thread by yielding on idle_frame
	yield(get_tree(),"idle_frame")
	print("you are in the main thread again:"+str(OS.get_thread_caller_id()))
	

in the previous version of this code i did not have the "if not already in thread" if statement, you don't need it and can call yield(reattach_to_thread(),"completed") from within the thread without issue, I just wanted to avoid running the code if it wasn't needed.

This should be an inbuilt thing where you can create special threads which exist as "thread_hook_loops" if you can resolve the stability issues of #67785 which are concerning.

@KnightNine
Copy link
Author

KnightNine commented Aug 30, 2023

This doesn't seem to be an issue in 4.X.

func thread_func():
	print("test")
var thread = Thread.new()
func _process(delta):
	thread.wait_to_finish()
	var c = Callable(self,"thread_func")
	thread.start(c)

Does not seem to stutter.


This is what the thread hooking code looks like in 4.X:

var thread = Thread.new()
var semaphore = Semaphore.new()
var thread_active = true
signal thread_frame()
#in the case where the yield() is called after emit_signal("thread_frame"), this is needed to ensure that eventually the yield catches the signal
var yielding_on_thread_process = false


func thread_process():
	Thread.set_thread_safety_checks_enabled(false)
	while thread_active:
		
		if !yielding_on_thread_process:
			print("held")
			semaphore.wait()
			print("released")
		#this fixes the deadlocking and error issues (because i think the thread will call emit_signal() at the exact same time as yield() is called in reattach_to_thread(), which seems to break godot ) (maybe make the delay a bit larger than 1 usec just in case):
		OS.delay_usec(100)
		emit_signal("thread_frame")
		
		

#this function needs to be called like this in order to work properly: 
""" 
var func_state = reattach_to_thread()
if func_state is GDScriptFunctionState:
	yield(func_state,"completed")
"""
#you could call it like this `yield(reattach_to_thread(),"completed")` but it will crash if you are already in the thread

func reattach_to_thread():
	var id = thread.get_id()
	#if not already in thread
	if OS.get_thread_caller_id() != int(id):
		#on semaphore.post(), the thread is "released" to emit a signal
		yielding_on_thread_process = true
		print("posted")
		semaphore.post()
		#this yield merges/hooks the function into the thread (yielding on anything adds the code being run to whatever thread is being yielded on)
		await self.thread_frame
		print("hooked")
		yielding_on_thread_process = false
	else:
		print("reattach not needed")
	

#HOW TO USE THESE FUNCTIONS:
# Called when the node enters the scene tree for the first time.
func _ready():
	
	#start the thread process
	var c = Callable(self,"thread_process")
	thread.start(c)
	
	print("you are currently in the main thread:"+str(OS.get_thread_caller_id()))
	
	
	await reattach_to_thread()
	
	print("you are in the thread now:"+str(OS.get_thread_caller_id()))
	
	#return to main thread by yielding on process_frame
	await get_tree().process_frame
	print("you are in the main thread again:"+str(OS.get_thread_caller_id()))

await makes it as simple as a single line to return to the thread.

I have yet to test if the stability issue still occurs when OS.delay_usec() isn't used

I still intend to use this as opposed to creating a new thread every time I want to run code from the thread.
This way I don't need to restructure my code around threads and can just switch back and forth between the thread and the main thread when needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants