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

Hl performance issues #8699

Closed
sonygod opened this issue Aug 26, 2019 · 14 comments
Closed

Hl performance issues #8699

sonygod opened this issue Aug 26, 2019 · 14 comments
Labels
Milestone

Comments

@sonygod
Copy link

@sonygod sonygod commented Aug 26, 2019

just compare this code run in Neko and HashLink

Neko target only cost <1% CPU

image

but HashLink cost more than 99% cpu.

image

   package;
import haxe.MainLoop;

using Lambda;

class Main {
	static function main() {
		trace("Hello, world!");

		MainLoop.addThread(function() {
			while (true) {
				Sys.sleep(10);
				trace(" hi");
			}
		
			
		});
	
	}
}
@sonygod

This comment has been minimized.

Copy link
Author

@sonygod sonygod commented Aug 26, 2019

Linux Ubuntu 18.04

image

@Aurel300

This comment has been minimized.

Copy link
Contributor

@Aurel300 Aurel300 commented Aug 26, 2019

It looks like HL is busy-waiting there. Will be fixed with new threads in 4.1.

@sonygod

This comment has been minimized.

Copy link
Author

@sonygod sonygod commented Aug 26, 2019

@Aurel300 will you fix as soon as possible? because current our game server uses hl as the Main target.

@RealyUniqueName RealyUniqueName added this to the Release 4.1 milestone Aug 26, 2019
@Aurel300

This comment has been minimized.

Copy link
Contributor

@Aurel300 Aurel300 commented Aug 26, 2019

I can't see what the bug really is. The sys_sleep implementation in HashLink seems fine, not busy-waiting. Could be related to MainLoop @ncannasse

@jonasmalacofilho

This comment has been minimized.

Copy link
Member

@jonasmalacofilho jonasmalacofilho commented Aug 26, 2019

The issue seems to be on the main thread.

No idea what goes on there, but taking a quick look I see that MainLoop (or more specifically EntryPoint) calls Lock.wait and busy waits there.

	public function wait(?timeout:Float):Bool {
		if (timeout == null) {
			deque.pop(true);
			return true;
		}
		var targetTime = haxe.Timer.stamp() + timeout;
		do {
			if (deque.pop(false) != null) {
				return true;
			}
		} while (haxe.Timer.stamp() < targetTime);
		return false;
	}

/std/hl/_std/sys/thread/Lock.hx#L33

Another thing I noticed is that MainLoop.tick will return 1e9 "ticks" if there are no events left, and that value ends up being used as the timeout for Lock.wait in this example. But I don't know if/how this matters.

@ncannasse

This comment has been minimized.

Copy link
Member

@ncannasse ncannasse commented Aug 26, 2019

Yes I confirm the issue comes from invalid sys.thread.Lock implementation in HL.
Fixing it would requiring porting the lock_* primitives from neko here (https://github.com/HaxeFoundation/neko/blob/master/libs/std/thread.c#L336) to HL sources here (https://github.com/HaxeFoundation/hashlink/blob/master/src/std/thread.c)

@sonygod

This comment has been minimized.

Copy link
Author

@sonygod sonygod commented Aug 27, 2019

@ConstNW will you have a look this issue?

@sonygod

This comment has been minimized.

Copy link
Author

@sonygod sonygod commented Aug 29, 2019

@Aurel300 @ncannasse why put this issue to 4.1? why not be fixed as soon as possible?

@sonygod

This comment has been minimized.

Copy link
Author

@sonygod sonygod commented Sep 3, 2019

thank you so much, it's fixed . and I need more tests and confirm to close.
image

@sonygod

This comment has been minimized.

Copy link
Author

@sonygod sonygod commented Sep 4, 2019

@ConstNW it seems only improve on Linux performance, but Windows still cost lot's CPU

and *.hl build from Windows then run in Linux still cost CPU like before?

@ConstNW

This comment has been minimized.

Copy link
Contributor

@ConstNW ConstNW commented Sep 4, 2019

@sonygod, no. on windows problem also fixed. try to compile with flag -D hl-ver=1.11

@sonygod

This comment has been minimized.

Copy link
Author

@sonygod sonygod commented Sep 4, 2019

but current Hashlink version is 1.10 only.
C:\HaxeToolkit\haxe\std/hl/_std/String.hx:211: characters 8-14 : Invalid version string "1.11". Should follow SemVer.

@Aurel300

This comment has been minimized.

Copy link
Contributor

@Aurel300 Aurel300 commented Sep 4, 2019

I think you might have to specify -D hl-ver=1.11.0 ?

@sonygod

This comment has been minimized.

Copy link
Author

@sonygod sonygod commented Sep 5, 2019

@Aurel300 yes, it's ok

@sonygod sonygod closed this Sep 5, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants
You can’t perform that action at this time.