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. #24869

Closed
Ploppy3 opened this issue Jan 9, 2019 · 26 comments
Closed

Godot stutters when starting a thread. #24869

Ploppy3 opened this issue Jan 9, 2019 · 26 comments

Comments

@Ploppy3
Copy link

Ploppy3 commented Jan 9, 2019

Godot version:

3.1-beta1
also tested on 3.0.6

OS/device including version:

Windows 10
GTX 1060
i5 2500k

Issue description:

Every time I start a thread, the game stutters, which defeats the purpose of using a thread.
Considering I'm new to threading the problem may be caused by my implementation.

image

Steps to reproduce:

var thread_test = Thread.new()
	
func test_thread(myvar):
	call_deferred('on_test_done')
	return null
	
func on_test_done():
	thread_test.wait_to_finish()
	print('thread done')
	
func _input(event):
	if event.is_action_pressed('ui_accept'):
		print('starting thread')
		thread_test.start(self, 'test_thread', null)

Minimal reproduction project:

test-threading.zip

@NathanWarden
Copy link
Contributor

It says the zip file cannot be found. It might be the thread_test.wait_to_finish() that's causing the hickup since that's called on the main thread, thus blocking the main thread until it finishes. Have you maybe tried polling for is_active() in the process function instead?

@NathanWarden
Copy link
Contributor

I might be doing something wrong, but I'm having the same issue. Below is my take on the code. This should be running butter smooth since it should be using the same thread as opposed to spinning up new ones. I'm more of a C# guy, so I might be doing something wrong. I'm going to try and see if calling wait_to_finish in a separate thread helps.

extends Node

var thread_test = Thread.new()
var isDone = false

func _ready():
	thread_test.start(self, 'test_thread', null)

func _process(delta):
	if isDone:
		on_test_done()
		isDone = false

	if !thread_test.is_active():
		debug(str(thread_test.is_active()))
		thread_test.start(self, 'test_thread', null)

func test_thread(myvar):
	var value = 0
	debug("start thread")
	for i in range(0, 5000000):
		value = i
	isDone = true
	return value

func on_test_done():
	var result = thread_test.wait_to_finish()
	debug('thread done' + str(result))

func debug(debugstr):
	#print (debugstr)
	pass

@Ploppy3
Copy link
Author

Ploppy3 commented Jan 9, 2019

I just followed this demo project from the Godot team.

How I understand it:

  • You .start() the thread
  • It runs the given method which ends with a deferred callback
  • The ..._done() method is then called, but only when the thread has finished its job

What do you think?

Edit: Updated the demo project

@NathanWarden
Copy link
Contributor

@Ploppy3 I think you're doing everything right and I think you found a bug.

So in this test, it shows something is definitely going on. The finish_thread is a one off thread that manages the thread_test thread. So, the worker thread is both started and finished in another thread instead of the main thread, yet there's still noticeable visual stuttering. To verify the below code is working simply uncomment the print statement at the bottom.

I wish I had more time to dig into this, but my play time is now over and I need to get back to my real job :)

extends Node

var thread_test = Thread.new()
var finish_thread = Thread.new()
var count = 0
var prevCount = 0

func _ready():
	thread_test.start(self, 'test_thread', null)
	finish_thread.start(self, 'on_test_done', null)

func _process(delta):
	if count > prevCount:
		debug(count)
		prevCount = count

func test_thread(myvar):
	var value = 0
	debug("start thread")
	for i in range(0, 5000000):
		value = i
	count += 1
	return value

func on_test_done(nullvar):
	while true:
		check_thread()
		pass
	pass

func check_thread():
	var result = thread_test.wait_to_finish()
	debug(result)

	if !thread_test.is_active():
		thread_test.start(self, 'test_thread', null)

func debug(debugstr):
	#print (debugstr)
	pass

@NathanWarden
Copy link
Contributor

NathanWarden commented Jan 9, 2019

The only solution that you might be able to do is manage when you're done with your thread yourself (in an array or something), and just never call wait_to_finish(). Or put another way, just have it in an endless while loop that triggers on a particular variable you set. This can get a bit hairy due to race conditions and such, so keep that in mind.

Pseudo:

func run_thread(): # This would get called with thread.start most likely in the _ready func
    while true:
        if has_stuff_to_do:
            do_stuff()
            has_stuff_to_do = false

func _input():
    if got_input():
        has_stuff_to_do = true

@bojidar-bg
Copy link
Contributor

Can't confirm on linux using the sample project, potentially a windows-only problem.
profiler graph
Arch Linux, Godot 0e25c32, i7-4790.

@NathanWarden
Copy link
Contributor

NathanWarden commented Jan 9, 2019

I can confirm with @bojidar-bg that it doesn't affect Linux. I'm on Pop OS 18.04. Edit: The same code I posted runs butter smooth.

@4ctarus
Copy link

4ctarus commented Jan 9, 2019

I can confirm the problem on windows.

Windows 10, amd fx-8350, gtx770

@NathanWarden
Copy link
Contributor

I just checked on my Mac which is a 2008 Mac Pro (Cheesegrater) running High Sierra. Everything looks good there too. So, it must be a Windows bug. I was running Windows 10 also.

@ghost
Copy link

ghost commented Jan 10, 2019

Recently did a bunch of experiments with threading and can confirm as well on Windows 10 I have been having spikes and frame loss issues with running threads. Was thinking it was because of the interactive loaders I was using, but found that just kicking a bunch of threads can cause severe drops in frame rate for very minor tasks.

I was hoping to take advantage of doing background work ahead of time to reduce load times, but at present it may not be viable.

@akien-mga akien-mga added this to the 3.1 milestone Jan 10, 2019
@Ploppy3 Ploppy3 closed this as completed Jan 10, 2019
@Ploppy3 Ploppy3 reopened this Jan 10, 2019
@Ploppy3
Copy link
Author

Ploppy3 commented Jan 10, 2019

@akien-mga thank you for setting the milestone to 3.1! Much appreciated!

@starry-abyss
Copy link
Contributor

Win 7 64 bit + i5-3570, spikes are smaller than on your picture, but are present still:

default

@dragmz
Copy link
Contributor

dragmz commented Jan 11, 2019

It seems to be SwapBuffers(hDC) call in ContextGL_Win::swap_buffers() that takes the extra frame idle time when using threads.

@reduz
Copy link
Member

reduz commented Jan 18, 2019

By the way you MUST use wait_to_finish after exiting a thread, else the thread is never freed. This is a Windows thing. I wonder if there is a better way to do it nowadays though.

And yes, the stutter due to this is normal on Windows, not sure if much else can be done.

@reduz
Copy link
Member

reduz commented Jan 18, 2019

In general, the best way to use threads is to start them and leave them running and give data to them via semaphore.

@Ploppy3
Copy link
Author

Ploppy3 commented Jan 18, 2019

I'm quite confused, do you mean that Godot will always freeze when starting a thread?

Also, how can I reuse a thread when I need to call:

  • start to call a method which start a new thread
  • wait_to_finish to get what the method returns which ends the thread

Edit: I originally thought I was reusing my thread by calling .new() once and .start() multiple times. But according to the doc .start() method "Starts a new Thread" so I don't know how to reuse a thread.

@ghost
Copy link

ghost commented Jan 18, 2019

New to this, and not sure either, as it at least seems that the Thread object just handles threads in some invisible way, and is not a thread itself. Wouldn't the start and wait still be an issue?

Is there an example out there of how to safely reuse a Godot thread over the duration of a game?

@neikeq
Copy link
Contributor

neikeq commented Jan 18, 2019

@reduz This is how wait_to_finish (aka. join) is implemented on Windows:

void ThreadWindows::wait_to_finish_func_windows(Thread *p_thread) {
ThreadWindows *tp = static_cast<ThreadWindows *>(p_thread);
ERR_FAIL_COND(!tp);
WaitForSingleObject(tp->handle, INFINITE);
CloseHandle(tp->handle);
//`memdelete(tp);
}

I don't think joining the thread is required to avoid leaking. AFAIK it would be enough to just close the handle. Have a look at this BSD-licensed implementation of std::thread for MinGW (specifically the detach() function): https://github.com/meganz/mingw-std-threads/blob/master/mingw.thread.h

dragmz added a commit to dragmz/godot that referenced this issue Jan 18, 2019
Fixes godotengine#24869 stuttering on Windows by reusing long running threadpool threads instead of creating a new thread on each call to Thread::start.
@Ploppy3
Copy link
Author

Ploppy3 commented Jan 21, 2019

So just to make sure, this PR is not fixing the thread creation stutter, but instead let Godot reuse existing threads.

I don't think the issue should be closed, what do you think?

@bojidar-bg
Copy link
Contributor

bojidar-bg commented Jan 21, 2019

Is there a way to make it better on windows? If there is a way which works as good as on unix, we might use that instead, otherwise we can just mention it as a platform limitation in the docs.

@Ploppy3
Copy link
Author

Ploppy3 commented Jan 21, 2019

Is it a generalized problem? I never heard of this before.

@dragmz
Copy link
Contributor

dragmz commented Jan 22, 2019

#25213 is an alternative fix that uses a dedicated thread pool (so a minimum number of pre-allocated worker threads can be set)

@Ploppy3
Copy link
Author

Ploppy3 commented Jan 22, 2019

@dragmz I can confirm your first fix complete solve the problem since I'm now reusing the thread. Went from unplayable to constant 60 fps when running many nav_agent on a big aStar grid.

@dragmz
Copy link
Contributor

dragmz commented Jan 22, 2019

@Ploppy3 Good to hear that!

I've noticed a slightly lower FPS when the first Godot thread call is made which I believe is because a new thread is added to the default thread pool.

The latter fix uses pre-allocated threads so it should eliminate the issue.

@Ploppy3
Copy link
Author

Ploppy3 commented Jan 22, 2019

I really like the performance boost so I don't mind the second fix being pushed to 3.2! Really quick and great job! Thank you!

slapin pushed a commit to slapin/godot that referenced this issue Jan 28, 2019
Fixes godotengine#24869 stuttering on Windows by reusing long running threadpool threads instead of creating a new thread on each call to Thread::start.
@KnightNine
Copy link

KnightNine commented Oct 10, 2022

Has there been a regression on this issue? There's a very noticeable stutter upon calling start() on a thread.

Here's some code to test it:

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

this issue is really getting in the way of what I'm trying to do here

update:
Running the same code in the release build still has a stutter:
release build

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