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

delay in keep_alive due to sleep #1969

Closed
berthubert opened this issue Oct 29, 2024 · 5 comments
Closed

delay in keep_alive due to sleep #1969

berthubert opened this issue Oct 29, 2024 · 5 comments

Comments

@berthubert
Copy link

Hi,

I have a new user for https://github.com/berthubert/bagconv and they are trying to send loads of queries to my cpp-httplib powered server, and getting timeouts.

I'm still investigating exactly what is going on, but I wonder what this is doing in keep_alive:

std::this_thread::sleep_for(microseconds{interval_usec});

There is already a call to select/poll that will sleep for interval_usec. What does this add, except latency for clients?
I'm also a bit confused why this has to loop so quickly, is this to catch a change in svr_sock to INVALID_SOCKET?

I've removed the sleep_for locally and things appear to be snappier now, and I don't see any problems. But perhaps I am missing something!

Bert
@yhirose
Copy link
Owner

yhirose commented Oct 29, 2024

This is to avoid unresponsiveness when there are more active threads than cpu cores in a machine.

I made an example to reproduce the problem in the following branch.
https://github.com/yhirose/cpp-httplib/tree/disable_sleep_for

If I build example/ssesvr and example/ssecli and run both on my machine (8 cpu cores), some of clients cannot receive events from the server. But If I get the std::this_thread::sleep_for call back, then all of clients can receive events from the server.

It indicates that if there are more active threads running on a machine with a cpp-httplib server, we might encounter issues. Using std::this_thread::sleep_for in the busy loop can allow other threads to execute.

Hope it helps.

@yhirose yhirose closed this as completed Oct 29, 2024
@berthubert
Copy link
Author

Thank you for investigating this! As far as I understand sockets and threads, the sleep_for should not be necessary and I wonder if it is in fact papering over a MacOS bug or perhaps a bug somewhere else. Here on Linux all ssecli threads receive updates with and without the sleep_for call. I added some code to check if all threads get data:

  vector<atomic<time_t>> times(threead_count);
  
  for (size_t i = 0; i < threead_count; i++) {
    threads.push_back(std::thread{[&, i] {
      httplib::Client("http://localhost:1234")
          .Get("/event1", [&, i](const char *data, size_t data_length) {
            if (i == threead_count - 1) {
              std::cout << string(data, data_length);

            }
            times[i]=time(0);            
            return true;
          });
    }});
  }

  for(;;) {
    time_t now = time(0);
    for(size_t i = 0 ; i < threead_count; ++i) {
      int delta = now - times[i];
      if(delta > 2)
        std::cout << "Thread " << i << " is behind by " << delta<<" seconds!\n";
    }
    sleep(1);
  }

However, I do get strange 'starvation' issues in other circumstances, even with the sleep in there. I'll investigate some more.

@berthubert
Copy link
Author

so I suspect that the loop of 10 millisecond select/poll calls is causing thread starvation on MacOS, where it keeps picking the same thread for getting CPU time. Increasing the CPPHTTPLIB_KEEPALIVE_TIMEOUT_CHECK_INTERVAL_USECOND might also make the problem go away. Perhaps.

@berthubert
Copy link
Author

Thank you!

@yhirose
Copy link
Owner

yhirose commented Nov 17, 2024

Thanks for your help!

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

No branches or pull requests

2 participants