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

NSRunLoop + dispatch_main_queue = 100% CPU --- SOLVED #378

Open
sheffler opened this issue Mar 5, 2024 · 8 comments
Open

NSRunLoop + dispatch_main_queue = 100% CPU --- SOLVED #378

sheffler opened this issue Mar 5, 2024 · 8 comments

Comments

@sheffler
Copy link

sheffler commented Mar 5, 2024

(On Linux.)

In programs using AppKit, GUI operations run on the main thread. With libdispatch, it's nice to be able to run GUI ops on the main thread using dispatch syntax

   dispatch_async(dispatch_get_main_queue(), ^{
       doSomethingGui();
       });

What I observed when I used such a construct with AppKit was that after the first
dispatch_async(dispatch_get_main_queue() ...) callback occurs the CPU usage would go
to 100%.

This is using

I had some time to work on this and believe I have solved the problem. NSRunLoop does integrate libdispatch, but does not seem to address what I'm describing.

There is the main NSRunLoop and a subordinate dispatch main run loop. My theory was that "dispatch" was signaling that its runloop needed to run with a file descriptor, and that when it was woken up, the FD was never "cleared", so that NSRunLoop always woke up GSMainQueueDrainer because it thought here was work to do in the dispatch run loop, even when there wasn't.

I believe that the way the dispatch main queue"notifies" the FD is through the function _dispatch_runloop_queue_class_poke(), which calls eventfd_write(fd, 1).

(See

The corresponding call to "receive" this event is eventfd_read(fd, &value)
but this is never called in the libdispatch code. So I believe that the event is "signalled" but never "cleared" and so NSRunLoop keeps trying to satisfy this notification.

I modified the following method to receive the event, and also "clear" the notification by reading it. With the modification, a simple test program that mixes NSRunLoop using AppKit and libdispatch works correctly now. (this small demo suffices https://github.com/sheffler/gnustep-rowcol-layout-examples/tree/main/10-manual-layout)

Tested on Debian 12 Bookworm and Ubuntu 22. Observed 100% CPU without the modification, and low CPU use with it.

I can prepare a pull-request, but i'd like some feedback too if anyone has some.

=== NSRunLoop.m ================

@implementation GSMainQueueDrainer
- (void) receivedEvent: (void*)data
		  type: (RunLoopEventType)type
		 extra: (void*)extra
	       forMode: (NSString*)mode
{
#if HAVE_DISPATCH_MAIN_QUEUE_DRAIN_NP
  dispatch_main_queue_drain_np();
#elif HAVE__DISPATCH_MAIN_QUEUE_CALLBACK_4CF
  // NEW CODE HERE
  uint64_t value;
  int fd = (int)(intptr_t)data;
  int n = eventfd_read(fd, &value);
  fprintf(stderr, "DEBUG EVENT: fd:%d,%lu %d\n", fd, value, n);
  // END NEW CODE
  _dispatch_main_queue_callback_4CF(NULL);
#else
#error libdispatch missing main queue callback function
#endif
}
@end

================================================

@triplef
Copy link
Member

triplef commented Mar 7, 2024

I believe calling eventfd_read() at that point is the right solution, we do the exact same in our custom integration of the Qt run loop with NSRunLoop and libdispatch. I think it only needs to be done on Linux platforms, not on Windows.

@sheffler
Copy link
Author

sheffler commented Mar 8, 2024

Thanks for the feedback. It's good to know a similar solution has been done on your other project using libdispatch.

I'll put in the appropriate #ifdef LINUX_ so that this only happens on Linux.

@triplef
Copy link
Member

triplef commented Mar 8, 2024

Sounds good. Not sure if LINUX includes platforms like Android, so maybe #ifndef _WIN32 would be better?

@rfm
Copy link
Contributor

rfm commented Mar 8, 2024

LINUX definitely doesn't cover the BSD's, so _WIN32 sounds good.

@sheffler
Copy link
Author

sheffler commented Mar 9, 2024

The definition of the FD is protected by linux in dispatch/queue.c, I believe because eventfd() is linux-specific (doesn't exist elsewhere). So the corresponding dispatch code only gets executed for defined(linux).

#elif defined(__linux__)
	int fd = eventfd(0, EFD_CLOEXEC | EFD_NONBLOCK);

@sheffler
Copy link
Author

sheffler commented Mar 9, 2024

Here's what i'm thinking the fix will look like

@implementation GSMainQueueDrainer
- (void) receivedEvent: (void*)data
		  type: (RunLoopEventType)type
		 extra: (void*)extra
	       forMode: (NSString*)mode
{
#if HAVE_DISPATCH_MAIN_QUEUE_DRAIN_NP
  dispatch_main_queue_drain_np();
#elif HAVE__DISPATCH_MAIN_QUEUE_CALLBACK_4CF

#if defined(__linux__)
  uint64_t value;
  int fd = (int)(intptr_t)data;
  int n = eventfd_read(fd, &value);
  (void) n;
#endif

  _dispatch_main_queue_callback_4CF(NULL);
#else
#error libdispatch missing main queue callback function
#endif
}
@end

@rfm
Copy link
Contributor

rfm commented Mar 9, 2024

I think that's probably a mistake in dispatch/queue.c since eventfd is not linux specific - at least one BSD system has it too. That may mean that the libdispatch implementation doesn't support BSD unix etc, but if/when it does that code would need to change.

I suppose the portable way to handle this would be with an autoconf test to determine how libdispatch is actually operating. For instance it might be using an eventfd on some operating systems and a pipe on other operating systems, so the code might need to use read() (to read from a pipe), and I suppose the number of bytes to be read might vary between systems too.

@sheffler
Copy link
Author

sheffler commented Mar 9, 2024

Here's the part that uses the FD with eventfd_write.

If and when other OSs are handled, the watcher would need to handle them too. But for now, linux can operate correctly.

static inline void
_dispatch_runloop_queue_class_poke(dispatch_lane_t dq)
{
	dispatch_runloop_handle_t handle = _dispatch_runloop_queue_get_handle(dq);
	if (!_dispatch_runloop_handle_is_valid(handle)) {
		return;
	}

	_dispatch_trace_runtime_event(worker_request, dq, 1);
#if HAVE_MACH
	mach_port_t mp = handle;
	kern_return_t kr = _dispatch_send_wakeup_runloop_thread(mp, 0);
	switch (kr) {
	case MACH_SEND_TIMEOUT:
	case MACH_SEND_TIMED_OUT:
	case MACH_SEND_INVALID_DEST:
		break;
	default:
		(void)dispatch_assume_zero(kr);
		break;
	}
#elif defined(__linux__)
	int result;
	do {
		result = eventfd_write(handle, 1);
	} while (result == -1 && errno == EINTR);
	(void)dispatch_assume_zero(result);
#elif defined(_WIN32)
	BOOL bSuccess;
	bSuccess = SetEvent(handle);
	(void)dispatch_assume(bSuccess);
#else
#error "runloop support not implemented on this platform"
#endif
}

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

No branches or pull requests

3 participants