After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 732439 - GSocket: avoid unnecessary g_socket_wait_condition() for blocking sockets
GSocket: avoid unnecessary g_socket_wait_condition() for blocking sockets
Status: RESOLVED DUPLICATE of bug 741707
Product: glib
Classification: Platform
Component: network
2.41.x
Other All
: Normal minor
: ---
Assigned To: gtkdev
gtkdev
Depends on:
Blocks:
 
 
Reported: 2014-06-29 18:25 UTC by Tim-Philipp Müller
Modified: 2015-01-17 14:08 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
gsocket: don't check socket state on blocking sockets on the first attempt (4.13 KB, patch)
2014-06-29 18:25 UTC, Tim-Philipp Müller
needs-work Details | Review

Description Tim-Philipp Müller 2014-06-29 18:25:22 UTC
Created attachment 279550 [details] [review]
gsocket: don't check socket state on blocking sockets on the first attempt

For GSockets set to be blocking we simulate the blocking with a pattern like this:

  while (TRUE)
    {
      if (socket->priv->blocking &&
          !g_socket_condition_wait (socket, G_IO_IN, cancellable, error))
        return -1;

      if ((res = do_something (socket, ...)) < 0)
        {
           if (socket->priv->blocking && error == EAGAIN)
              continue;
           ...
        }
    }

This means we run the g_socket_condition_wait() even in the first loop iteration before even trying to do anything on the socket. This seems unnecessary and should only be done once we got back an EAGAIN after trying.

The attached patch skips the wait in the first loop iteration. It uses the results variable to check whether it's the first iteration. I don't know if you would prefer that or if you would prefer a separate gboolean first variable for clarity.
Comment 1 Dan Winship 2014-07-05 14:57:57 UTC
i think it would be clearer if you rearranged the code more. maybe something like:

  while ((res = do_something (socket, ...)) < 0)
    {
       if (errno == EINTR)
         continue;
       if (socket->priv->blocking && errno == EAGAIN)
         {
           if (g_socket_condition_wait (socket, G_IO_IN, cancellable, error))
             continue;
           else
             return -1;
         }

       g_set_error (...);
       return -1;
    }
Comment 2 Tim-Philipp Müller 2014-07-16 14:56:18 UTC
Comment on attachment 279550 [details] [review]
gsocket: don't check socket state on blocking sockets on the first attempt

Thanks for the review, will fix as you suggest.
Comment 3 Paolo Borelli 2015-01-05 17:02:50 UTC
This is basically a dupe of #741707 which has an updated patch.

As described in the other bug, this is not only an optimization, but it is also required on win32 when you create the gsocked from an existing fd, since the fd is not notified to be writable until you try to write into it
Comment 4 Paolo Borelli 2015-01-17 14:08:58 UTC
Marking as duplicate since the other patch was pushed

*** This bug has been marked as a duplicate of bug 741707 ***