View Issue Details
| ID | Project | Category | View Status | Date Submitted | Last Update |
|---|---|---|---|---|---|
| 0002599 | GNUnet | util library | public | 2012-10-25 02:04 | 2012-11-05 18:33 |
| Reporter | bratao | Assigned To | |||
| Priority | normal | Severity | tweak | Reproducibility | always |
| Status | closed | Resolution | fixed | ||
| Platform | W32 | OS | Windows | OS Version | 8 |
| Product Version | Git master | ||||
| Target Version | 0.9.4 | Fixed in Version | 0.9.4 | ||
| Summary | 0002599: Improve W32 Select performance | ||||
| Description | Our W32 is too slow, and is a major bottleneck, especially on high-tranfer rate scenarios. After a lot of profiling, this patch do a small modification that heavily increase the performance to me ( 80% CPU usage reduction and 400%(!) throughput increase ). What it do: -Do the select with 0 timeout to all cases. The impact if is wrong is very small, the gains are huge if it got something. -If the previous select returned something,or is a request with 0 timeout. Process the pipes and return. -Else load our old method (heavy weapons) This works, because the GNUNet scheduler calling SELECT with 0 timeout is a VERY common scenario. And we prevent having to load all our MASSIVE code for muxing pipes and network handles. All the tests are passing with this patch. | ||||
| Tags | No tags attached. | ||||
| Attached Files | network.c.patch (6,651 bytes)
Index: network.c
===================================================================
--- network.c (revision 24495)
+++ network.c (working copy)
@@ -1404,6 +1404,194 @@
FD_COPY (&efds->sds, &bexcept);
#endif
}
+
+
+ //Run the select for all the cases. Is cheap and can save us to load all the heavy weapons.
+ int selectret = 0;
+ if(rfds || wfds || efds)
+ {
+ //Do the select now
+ select_timeout.tv_sec = 0;
+ select_timeout.tv_usec = 0;
+
+
+ selectret = select (1, &aread, &awrite, &aexcept, &select_timeout);
+ for (i = 0; i < awrite.fd_count; i++)
+ {
+ if (awrite.fd_array[i] != 0 && awrite.fd_array[i] != -1)
+ FD_SET (awrite.fd_array[i], &aexcept);
+ }
+
+ }
+
+ //If our select returned something or is a 0-timed request. Process the pipes and
+ // Get out of here !
+ if((selectret > 0) || (ms_total == 0) )
+ {
+ /* Read Pipes */
+ if (rfds && read_handles)
+ {
+ struct GNUNET_CONTAINER_SList_Iterator i;
+
+ for (i = GNUNET_CONTAINER_slist_begin (rfds->handles);
+ GNUNET_CONTAINER_slist_end (&i) != GNUNET_YES;
+ GNUNET_CONTAINER_slist_next (&i))
+ {
+ struct GNUNET_DISK_FileHandle *fh;
+
+ fh = (struct GNUNET_DISK_FileHandle *) GNUNET_CONTAINER_slist_get (&i,
+ NULL);
+ if (fh->type == GNUNET_DISK_HANLDE_TYPE_PIPE)
+ {
+ DWORD error;
+ BOOL bret;
+
+ SetLastError (0);
+ DWORD waitstatus = 0;
+ bret =
+ PeekNamedPipe (fh->h, NULL, 0, NULL, &waitstatus, NULL);
+ error = GetLastError ();
+ LOG (GNUNET_ERROR_TYPE_DEBUG,
+ "Peek at read pipe %d (0x%x) returned %d (%d bytes available) GLE %u\n",
+ i, fh->h, bret, waitstatus, error);
+ if (bret == 0)
+ {
+ /* TODO: either add more errors to this condition, or eliminate it
+ * entirely (failed to peek -> pipe is in serious trouble, should
+ * be selected as readable).
+ */
+ if (error != ERROR_BROKEN_PIPE && error != ERROR_INVALID_HANDLE)
+ continue;
+ }
+ else if (waitstatus <= 0)
+ continue;
+ GNUNET_CONTAINER_slist_add (handles_read,
+ GNUNET_CONTAINER_SLIST_DISPOSITION_TRANSIENT,
+ fh,
+ sizeof (struct GNUNET_DISK_FileHandle));
+ retcode++;
+ LOG (GNUNET_ERROR_TYPE_DEBUG, "Added read Pipe 0x%x (0x%x)\n",
+ fh, fh->h);
+ }
+ else
+ {
+ GNUNET_CONTAINER_slist_add (handles_read,
+ GNUNET_CONTAINER_SLIST_DISPOSITION_TRANSIENT,
+ fh, sizeof (struct GNUNET_DISK_FileHandle));
+ retcode++;
+ }
+ }
+ }
+ if (wfds && write_handles)
+ {
+ LOG (GNUNET_ERROR_TYPE_DEBUG,
+ "Adding the write ready event to the array as %d\n", nhandles);
+ GNUNET_CONTAINER_slist_append (handles_write, wfds->handles);
+ retcode += write_handles;
+ }
+ if (efds && ex_handles)
+ {
+ struct GNUNET_CONTAINER_SList_Iterator i;
+
+ for (i = GNUNET_CONTAINER_slist_begin (efds->handles);
+ GNUNET_CONTAINER_slist_end (&i) != GNUNET_YES;
+ GNUNET_CONTAINER_slist_next (&i))
+ {
+ struct GNUNET_DISK_FileHandle *fh;
+ DWORD dwBytes;
+
+ fh = (struct GNUNET_DISK_FileHandle *) GNUNET_CONTAINER_slist_get (&i,
+ NULL);
+ if (fh->type == GNUNET_DISK_HANLDE_TYPE_PIPE)
+ {
+ if (!PeekNamedPipe (fh->h, NULL, 0, NULL, &dwBytes, NULL))
+ {
+ GNUNET_CONTAINER_slist_add (handles_except,
+ GNUNET_CONTAINER_SLIST_DISPOSITION_TRANSIENT,
+ fh,
+ sizeof (struct GNUNET_DISK_FileHandle));
+ retcode++;
+ }
+ }
+ }
+ }
+
+
+
+
+
+ //All with our select result.
+ retcode+= selectret;
+
+
+ if (rfds)
+ {
+ GNUNET_NETWORK_fdset_zero (rfds);
+ if (retcode != -1)
+ GNUNET_NETWORK_fdset_copy_native (rfds, &aread, retcode);
+ GNUNET_CONTAINER_slist_append (rfds->handles, handles_read);
+ }
+ if (wfds)
+ {
+ GNUNET_NETWORK_fdset_zero (wfds);
+ if (retcode != -1)
+ GNUNET_NETWORK_fdset_copy_native (wfds, &awrite, retcode);
+ GNUNET_CONTAINER_slist_append (wfds->handles, handles_write);
+ }
+ if (efds)
+ {
+ GNUNET_NETWORK_fdset_zero (efds);
+ if (retcode != -1)
+ GNUNET_NETWORK_fdset_copy_native (efds, &aexcept, retcode);
+ GNUNET_CONTAINER_slist_append (efds->handles, handles_except);
+ }
+ GNUNET_CONTAINER_slist_destroy (handles_read);
+ GNUNET_CONTAINER_slist_destroy (handles_write);
+ GNUNET_CONTAINER_slist_destroy (handles_except);
+
+
+
+ //This should only happen on the 0-timed request !
+ if(retcode == -1) //
+ return 0;
+ else
+ return retcode;
+
+ }
+
+
+ //If we got until here, Load the heavy weapons !
+ retcode = 0;
+
+ FD_ZERO (&aread);
+ FD_ZERO (&awrite);
+ FD_ZERO (&aexcept);
+#if DEBUG_NETWORK
+ FD_ZERO (&bread);
+ FD_ZERO (&bwrite);
+ FD_ZERO (&bexcept);
+#endif
+ if (rfds)
+ {
+ FD_COPY (&rfds->sds, &aread);
+#if DEBUG_NETWORK
+ FD_COPY (&rfds->sds, &bread);
+#endif
+ }
+ if (wfds)
+ {
+ FD_COPY (&wfds->sds, &awrite);
+#if DEBUG_NETWORK
+ FD_COPY (&wfds->sds, &bwrite);
+#endif
+ }
+ if (efds)
+ {
+ FD_COPY (&efds->sds, &aexcept);
+#if DEBUG_NETWORK
+ FD_COPY (&efds->sds, &bexcept);
+#endif
+ }
/* We will first Add the PIPES to the events */
/* Read Pipes */
if (rfds && read_handles)
@@ -1715,6 +1903,7 @@
else
#endif
return 0;
+
}
/* end of network.c */
network.c.v2.patch (10,006 bytes)
Index: network.c
===================================================================
--- network.c (revision 24495)
+++ network.c (working copy)
@@ -91,13 +91,13 @@
return unixpath; /* no shortening required */
GNUNET_CRYPTO_short_hash (unixpath, slen, &sh);
while (sizeof (struct GNUNET_CRYPTO_ShortHashAsciiEncoded) +
- strlen (unixpath) >= upm)
+ strlen (unixpath) >= upm)
{
if (NULL == (end = strrchr (unixpath, '/')))
{
GNUNET_log (GNUNET_ERROR_TYPE_ERROR,
- _("Unable to shorten unix path `%s' while keeping name unique\n"),
- unixpath);
+ _("Unable to shorten unix path `%s' while keeping name unique\n"),
+ unixpath);
GNUNET_free (unixpath);
return NULL;
}
@@ -256,7 +256,7 @@
*/
static int
initialize_network_handle (struct GNUNET_NETWORK_Handle *h,
- int af, int type)
+ int af, int type)
{
h->af = af;
if (h->fd == INVALID_SOCKET)
@@ -326,8 +326,8 @@
#endif
ret->fd = accept (desc->fd, address, address_len);
if (GNUNET_OK != initialize_network_handle (ret,
- (NULL != address) ? address->sa_family : desc->af,
- SOCK_STREAM))
+ (NULL != address) ? address->sa_family : desc->af,
+ SOCK_STREAM))
return NULL;
return ret;
}
@@ -1199,27 +1199,15 @@
int nhandles = 0;
- static HANDLE hEventPipeWrite = 0;
- static HANDLE hEventReadReady = 0;
-
- static struct _select_params sp;
- static HANDLE select_thread = NULL;
- static HANDLE select_finished_event = NULL;
- static HANDLE select_standby_event = NULL;
- static SOCKET select_wakeup_socket = -1;
- static SOCKET select_send_socket = -1;
- static struct timeval select_timeout;
-
- int readPipes = 0;
- int writePipePos = 0;
-
- HANDLE handle_array[FD_SETSIZE + 2];
+
int returncode = -1;
int returnedpos = 0;
struct GNUNET_CONTAINER_SList *handles_read;
struct GNUNET_CONTAINER_SList *handles_write;
struct GNUNET_CONTAINER_SList *handles_except;
+
+ static struct timeval select_timeout;
fd_set aread;
fd_set awrite;
@@ -1231,8 +1219,7 @@
fd_set bexcept;
#endif
- /* TODO: Make this growable */
- struct GNUNET_DISK_FileHandle *readArray[50];
+
#else
struct timeval tv;
#endif
@@ -1320,8 +1307,226 @@
return 0;
}
- if (select_thread == NULL)
+
+
+ handles_read = GNUNET_CONTAINER_slist_create ();
+ handles_write = GNUNET_CONTAINER_slist_create ();
+ handles_except = GNUNET_CONTAINER_slist_create ();
+ FD_ZERO (&aread);
+ FD_ZERO (&awrite);
+ FD_ZERO (&aexcept);
+#if DEBUG_NETWORK
+ FD_ZERO (&bread);
+ FD_ZERO (&bwrite);
+ FD_ZERO (&bexcept);
+#endif
+ if (rfds)
{
+ FD_COPY (&rfds->sds, &aread);
+#if DEBUG_NETWORK
+ FD_COPY (&rfds->sds, &bread);
+#endif
+ }
+ if (wfds)
+ {
+ FD_COPY (&wfds->sds, &awrite);
+#if DEBUG_NETWORK
+ FD_COPY (&wfds->sds, &bwrite);
+#endif
+ }
+ if (efds)
+ {
+ FD_COPY (&efds->sds, &aexcept);
+#if DEBUG_NETWORK
+ FD_COPY (&efds->sds, &bexcept);
+#endif
+ }
+
+
+ /* Run the select for all the cases. Is cheap and can save us to load all the heavy weapons.
+ By profiling we detected that this is the 90% of the scenarios.
+ */
+ int selectret = 0;
+ if(rfds || wfds || efds)
+ {
+ /*Do the select now*/
+ select_timeout.tv_sec = 0;
+ select_timeout.tv_usec = 0;
+
+ /*Copy all the writes to the except, so we can detect connect() errors*/
+ for (i = 0; i < awrite.fd_count; i++)
+ {
+ if (awrite.fd_array[i] != 0 && awrite.fd_array[i] != -1)
+ FD_SET (awrite.fd_array[i], &aexcept);
+ }
+
+ selectret = select (1, &aread, &awrite, &aexcept, &select_timeout);
+
+ /* Check aexcept, add its contents to awrite */
+ for (i = 0; i < aexcept.fd_count; i++)
+ {
+ if (aexcept.fd_array[i] != 0 && aexcept.fd_array[i] != -1)
+ FD_SET (aexcept.fd_array[i], &awrite);
+ }
+
+ }
+
+ /*If our select returned something or is a 0-timed request. Process the pipes and get out of here !*/
+ if((selectret > 0) || (ms_total == 0) )
+ {
+ /* Read Pipes */
+ if (rfds && read_handles)
+ {
+ struct GNUNET_CONTAINER_SList_Iterator i;
+
+ for (i = GNUNET_CONTAINER_slist_begin (rfds->handles);
+ GNUNET_CONTAINER_slist_end (&i) != GNUNET_YES;
+ GNUNET_CONTAINER_slist_next (&i))
+ {
+ struct GNUNET_DISK_FileHandle *fh;
+
+ fh = (struct GNUNET_DISK_FileHandle *) GNUNET_CONTAINER_slist_get (&i,
+ NULL);
+ if (fh->type == GNUNET_DISK_HANLDE_TYPE_PIPE)
+ {
+ DWORD error;
+ BOOL bret;
+
+ SetLastError (0);
+ DWORD waitstatus = 0;
+ bret =
+ PeekNamedPipe (fh->h, NULL, 0, NULL, &waitstatus, NULL);
+ error = GetLastError ();
+ LOG (GNUNET_ERROR_TYPE_DEBUG,
+ "Peek at read pipe %d (0x%x) returned %d (%d bytes available) GLE %u\n",
+ i, fh->h, bret, waitstatus, error);
+ if (bret == 0)
+ {
+ /* TODO: either add more errors to this condition, or eliminate it
+ * entirely (failed to peek -> pipe is in serious trouble, should
+ * be selected as readable).
+ */
+ if (error != ERROR_BROKEN_PIPE && error != ERROR_INVALID_HANDLE)
+ continue;
+ }
+ else if (waitstatus <= 0)
+ continue;
+ GNUNET_CONTAINER_slist_add (handles_read,
+ GNUNET_CONTAINER_SLIST_DISPOSITION_TRANSIENT,
+ fh,
+ sizeof (struct GNUNET_DISK_FileHandle));
+ retcode++;
+ LOG (GNUNET_ERROR_TYPE_DEBUG, "Added read Pipe 0x%x (0x%x)\n",
+ fh, fh->h);
+ }
+ else
+ {
+ GNUNET_CONTAINER_slist_add (handles_read,
+ GNUNET_CONTAINER_SLIST_DISPOSITION_TRANSIENT,
+ fh, sizeof (struct GNUNET_DISK_FileHandle));
+ retcode++;
+ }
+ }
+ }
+ if (wfds && write_handles)
+ {
+ LOG (GNUNET_ERROR_TYPE_DEBUG,
+ "Adding the write ready event to the array as %d\n", nhandles);
+ GNUNET_CONTAINER_slist_append (handles_write, wfds->handles);
+ retcode += write_handles;
+ }
+ if (efds && ex_handles)
+ {
+ struct GNUNET_CONTAINER_SList_Iterator i;
+
+ for (i = GNUNET_CONTAINER_slist_begin (efds->handles);
+ GNUNET_CONTAINER_slist_end (&i) != GNUNET_YES;
+ GNUNET_CONTAINER_slist_next (&i))
+ {
+ struct GNUNET_DISK_FileHandle *fh;
+ DWORD dwBytes;
+
+ fh = (struct GNUNET_DISK_FileHandle *) GNUNET_CONTAINER_slist_get (&i,
+ NULL);
+ if (fh->type == GNUNET_DISK_HANLDE_TYPE_PIPE)
+ {
+ if (!PeekNamedPipe (fh->h, NULL, 0, NULL, &dwBytes, NULL))
+ {
+ GNUNET_CONTAINER_slist_add (handles_except,
+ GNUNET_CONTAINER_SLIST_DISPOSITION_TRANSIENT,
+ fh,
+ sizeof (struct GNUNET_DISK_FileHandle));
+ retcode++;
+ }
+ }
+ }
+ }
+
+
+
+
+
+ /*All with our select result.*/
+ retcode+= selectret;
+
+ if (rfds)
+ {
+ GNUNET_NETWORK_fdset_zero (rfds);
+ if (retcode != -1)
+ GNUNET_NETWORK_fdset_copy_native (rfds, &aread, retcode);
+ GNUNET_CONTAINER_slist_append (rfds->handles, handles_read);
+ }
+ if (wfds)
+ {
+ GNUNET_NETWORK_fdset_zero (wfds);
+ if (retcode != -1)
+ GNUNET_NETWORK_fdset_copy_native (wfds, &awrite, retcode);
+ GNUNET_CONTAINER_slist_append (wfds->handles, handles_write);
+ }
+ if (efds)
+ {
+ GNUNET_NETWORK_fdset_zero (efds);
+ if (retcode != -1)
+ GNUNET_NETWORK_fdset_copy_native (efds, &aexcept, retcode);
+ GNUNET_CONTAINER_slist_append (efds->handles, handles_except);
+ }
+ GNUNET_CONTAINER_slist_destroy (handles_read);
+ GNUNET_CONTAINER_slist_destroy (handles_write);
+ GNUNET_CONTAINER_slist_destroy (handles_except);
+
+
+
+ /*This should only happen on the 0-timed request*/
+ if(retcode == -1)
+ return 0;
+ else
+ return retcode;
+
+ }
+
+
+ /*If we got until here, Load the heavy weapons*/
+ static HANDLE hEventPipeWrite = 0;
+ static HANDLE hEventReadReady = 0;
+
+ static struct _select_params sp;
+ static HANDLE select_thread = NULL;
+ static HANDLE select_finished_event = NULL;
+ static HANDLE select_standby_event = NULL;
+ static SOCKET select_wakeup_socket = -1;
+ static SOCKET select_send_socket = -1;
+
+
+ int readPipes = 0;
+ int writePipePos = 0;
+
+ HANDLE handle_array[FD_SETSIZE + 2];
+
+ /* TODO: Make this growable */
+ struct GNUNET_DISK_FileHandle *readArray[50];
+
+ if (select_thread == NULL)
+ {
SOCKET select_listening_socket = -1;
struct sockaddr_in s_in;
int alen;
@@ -1371,10 +1576,9 @@
hEventPipeWrite = CreateEvent (NULL, TRUE, TRUE, NULL);
readPipes = 0;
writePipePos = -1;
-
- handles_read = GNUNET_CONTAINER_slist_create ();
- handles_write = GNUNET_CONTAINER_slist_create ();
- handles_except = GNUNET_CONTAINER_slist_create ();
+
+ retcode = 0;
+
FD_ZERO (&aread);
FD_ZERO (&awrite);
FD_ZERO (&aexcept);
@@ -1715,6 +1919,7 @@
else
#endif
return 0;
+
}
/* end of network.c */
network.c.v3.patch (8,379 bytes)
Index: network.c
===================================================================
--- network.c (revision 24495)
+++ network.c (working copy)
@@ -1198,7 +1198,7 @@
DWORD ms_total = 0;
int nhandles = 0;
-
+
static HANDLE hEventPipeWrite = 0;
static HANDLE hEventReadReady = 0;
@@ -1208,18 +1208,25 @@
static HANDLE select_standby_event = NULL;
static SOCKET select_wakeup_socket = -1;
static SOCKET select_send_socket = -1;
- static struct timeval select_timeout;
+
int readPipes = 0;
int writePipePos = 0;
HANDLE handle_array[FD_SETSIZE + 2];
+
+ /* TODO: Make this growable */
+ struct GNUNET_DISK_FileHandle *readArray[50];
+
int returncode = -1;
int returnedpos = 0;
struct GNUNET_CONTAINER_SList *handles_read;
struct GNUNET_CONTAINER_SList *handles_write;
struct GNUNET_CONTAINER_SList *handles_except;
+
+ static struct timeval select_timeout;
+ int selectret = 0;
fd_set aread;
fd_set awrite;
@@ -1231,8 +1238,7 @@
fd_set bexcept;
#endif
- /* TODO: Make this growable */
- struct GNUNET_DISK_FileHandle *readArray[50];
+
#else
struct timeval tv;
#endif
@@ -1320,8 +1326,207 @@
return 0;
}
- if (select_thread == NULL)
+
+
+ handles_read = GNUNET_CONTAINER_slist_create ();
+ handles_write = GNUNET_CONTAINER_slist_create ();
+ handles_except = GNUNET_CONTAINER_slist_create ();
+ FD_ZERO (&aread);
+ FD_ZERO (&awrite);
+ FD_ZERO (&aexcept);
+#if DEBUG_NETWORK
+ FD_ZERO (&bread);
+ FD_ZERO (&bwrite);
+ FD_ZERO (&bexcept);
+#endif
+ if (rfds)
{
+ FD_COPY (&rfds->sds, &aread);
+#if DEBUG_NETWORK
+ FD_COPY (&rfds->sds, &bread);
+#endif
+ }
+ if (wfds)
+ {
+ FD_COPY (&wfds->sds, &awrite);
+#if DEBUG_NETWORK
+ FD_COPY (&wfds->sds, &bwrite);
+#endif
+ }
+ if (efds)
+ {
+ FD_COPY (&efds->sds, &aexcept);
+#if DEBUG_NETWORK
+ FD_COPY (&efds->sds, &bexcept);
+#endif
+ }
+
+
+ /* Run the select for all the cases. Is cheap and can save us to load all the heavy weapons.
+ By profiling we detected that this is the 90% of the scenarios.
+ */
+
+ /*Do the select now*/
+ select_timeout.tv_sec = 0;
+ select_timeout.tv_usec = 0;
+
+ /*Copy all the writes to the except, so we can detect connect() errors*/
+ for (i = 0; i < awrite.fd_count; i++)
+ {
+ if (awrite.fd_array[i] != 0 && awrite.fd_array[i] != -1)
+ FD_SET (awrite.fd_array[i], &aexcept);
+ }
+ selectret = select (1, (rfds != NULL) ? &aread : NULL,
+ (wfds != NULL) ? &awrite : NULL,
+ &aexcept, &select_timeout);
+
+ /* Check aexcept, add its contents to awrite */
+ for (i = 0; i < aexcept.fd_count; i++)
+ {
+ if (aexcept.fd_array[i] != 0 && aexcept.fd_array[i] != -1)
+ FD_SET (aexcept.fd_array[i], &awrite);
+ }
+
+ /*If our select returned something or is a 0-timed request. Process the pipes and get out of here !*/
+ if((selectret > 0) || (ms_total == 0) )
+ {
+ /* Read Pipes */
+ if (rfds && read_handles)
+ {
+ struct GNUNET_CONTAINER_SList_Iterator i;
+
+ for (i = GNUNET_CONTAINER_slist_begin (rfds->handles);
+ GNUNET_CONTAINER_slist_end (&i) != GNUNET_YES;
+ GNUNET_CONTAINER_slist_next (&i))
+ {
+ struct GNUNET_DISK_FileHandle *fh;
+
+ fh = (struct GNUNET_DISK_FileHandle *) GNUNET_CONTAINER_slist_get (&i,
+ NULL);
+ if (fh->type == GNUNET_DISK_HANLDE_TYPE_PIPE)
+ {
+ DWORD error;
+ BOOL bret;
+
+ SetLastError (0);
+ DWORD waitstatus = 0;
+ bret =
+ PeekNamedPipe (fh->h, NULL, 0, NULL, &waitstatus, NULL);
+ error = GetLastError ();
+ LOG (GNUNET_ERROR_TYPE_DEBUG,
+ "Peek at read pipe %d (0x%x) returned %d (%d bytes available) GLE %u\n",
+ i, fh->h, bret, waitstatus, error);
+ if (bret == 0)
+ {
+ /* TODO: either add more errors to this condition, or eliminate it
+ * entirely (failed to peek -> pipe is in serious trouble, should
+ * be selected as readable).
+ */
+ if (error != ERROR_BROKEN_PIPE && error != ERROR_INVALID_HANDLE)
+ continue;
+ }
+ else if (waitstatus <= 0)
+ continue;
+ GNUNET_CONTAINER_slist_add (handles_read,
+ GNUNET_CONTAINER_SLIST_DISPOSITION_TRANSIENT,
+ fh,
+ sizeof (struct GNUNET_DISK_FileHandle));
+ retcode++;
+ LOG (GNUNET_ERROR_TYPE_DEBUG, "Added read Pipe 0x%x (0x%x)\n",
+ fh, fh->h);
+ }
+ else
+ {
+ GNUNET_CONTAINER_slist_add (handles_read,
+ GNUNET_CONTAINER_SLIST_DISPOSITION_TRANSIENT,
+ fh, sizeof (struct GNUNET_DISK_FileHandle));
+ retcode++;
+ }
+ }
+ }
+ if (wfds && write_handles)
+ {
+ LOG (GNUNET_ERROR_TYPE_DEBUG,
+ "Adding the write ready event to the array as %d\n", nhandles);
+ GNUNET_CONTAINER_slist_append (handles_write, wfds->handles);
+ retcode += write_handles;
+ }
+ if (efds && ex_handles)
+ {
+ struct GNUNET_CONTAINER_SList_Iterator i;
+
+ for (i = GNUNET_CONTAINER_slist_begin (efds->handles);
+ GNUNET_CONTAINER_slist_end (&i) != GNUNET_YES;
+ GNUNET_CONTAINER_slist_next (&i))
+ {
+ struct GNUNET_DISK_FileHandle *fh;
+ DWORD dwBytes;
+
+ fh = (struct GNUNET_DISK_FileHandle *) GNUNET_CONTAINER_slist_get (&i,
+ NULL);
+ if (fh->type == GNUNET_DISK_HANLDE_TYPE_PIPE)
+ {
+ if (!PeekNamedPipe (fh->h, NULL, 0, NULL, &dwBytes, NULL))
+ {
+ GNUNET_CONTAINER_slist_add (handles_except,
+ GNUNET_CONTAINER_SLIST_DISPOSITION_TRANSIENT,
+ fh,
+ sizeof (struct GNUNET_DISK_FileHandle));
+ retcode++;
+ }
+ }
+ }
+ }
+
+
+
+ /*All with our select result.*/
+ retcode+= selectret;
+
+
+
+ if (rfds)
+ {
+ GNUNET_NETWORK_fdset_zero (rfds);
+ if (selectret != -1)
+ GNUNET_NETWORK_fdset_copy_native (rfds, &aread, selectret);
+ GNUNET_CONTAINER_slist_append (rfds->handles, handles_read);
+ }
+ if (wfds)
+ {
+
+ GNUNET_NETWORK_fdset_zero (wfds);
+ if (selectret != -1)
+ GNUNET_NETWORK_fdset_copy_native (wfds, &awrite, selectret);
+ GNUNET_CONTAINER_slist_append (wfds->handles, handles_write);
+ }
+ if (efds)
+ {
+ GNUNET_NETWORK_fdset_zero (efds);
+ if (selectret != -1)
+ GNUNET_NETWORK_fdset_copy_native (efds, &aexcept, selectret);
+ GNUNET_CONTAINER_slist_append (efds->handles, handles_except);
+ }
+ GNUNET_CONTAINER_slist_destroy (handles_read);
+ GNUNET_CONTAINER_slist_destroy (handles_write);
+ GNUNET_CONTAINER_slist_destroy (handles_except);
+
+
+
+ /*This should only happen on the 0-timed request*/
+ if(retcode == -1)
+ return 0;
+ else
+ return retcode;
+
+ }
+
+
+ /*If we got until here, Load the heavy weapons*/
+
+
+ if (select_thread == NULL)
+ {
SOCKET select_listening_socket = -1;
struct sockaddr_in s_in;
int alen;
@@ -1371,10 +1576,9 @@
hEventPipeWrite = CreateEvent (NULL, TRUE, TRUE, NULL);
readPipes = 0;
writePipePos = -1;
-
- handles_read = GNUNET_CONTAINER_slist_create ();
- handles_write = GNUNET_CONTAINER_slist_create ();
- handles_except = GNUNET_CONTAINER_slist_create ();
+
+ retcode = 0;
+
FD_ZERO (&aread);
FD_ZERO (&awrite);
FD_ZERO (&aexcept);
@@ -1715,6 +1919,7 @@
else
#endif
return 0;
+
}
/* end of network.c */
| ||||
|
|
Ok, first, that awrite->aexcept block after select might also need aexcept->awrite block _before_ select. Look for "Failed connections cause sockets to be set in errorfds on W32" comment in the source. Other than that: 1) Fix comments to use /* */ instead of // 2) Fix indentation 3) Convert tabs to spaces and it's good to go! Do you have svn write permissions? |
|
|
Fixed ! Thank your for reviewing it ! I also moved some declarations around. I don't know if the compiler will really allocate those arrays in the middle of the function, or will allocate at the beginning of the function. I do not have svn write. |
|
|
Well I don't know if the GNUNet is adhering to ANSI C or C99. But there's variable declaration in the middle of the block. I moved for code clarity and for a possible (?) improvement, as allocating two big arrays for nothing is painful for a function called as much as Select. |
|
|
Aren't stack allocations supposed to be kind of free? |
|
|
According to http://stackoverflow.com/questions/161053/c-which-is-faster-stack-allocation-or-heap-allocation , stack allocations are free. It's bad to allocate a LOT on the stack (because stack is very finite), but these arrays don't seem to be too large to allocate on the stack. However, there's a TODO to make them growable (i.e. heap-allocated). |
|
|
I corrected the variable declarations. |
|
|
Instead of heap-allocating, you might also choose to first determine the needed size and then create a new scope for the stack allocation: size_t bla; bla = fun(); { char buf[size]; use (buf); } That is often the best solution (fast, no over-allocation). |
|
|
Fixed in r24553 |
| Date Modified | Username | Field | Change |
|---|---|---|---|
| 2012-10-25 02:04 | bratao | New Issue | |
| 2012-10-25 02:04 | bratao | File Added: network.c.patch | |
| 2012-10-25 07:26 | LRN | Note Added: 0006490 | |
| 2012-10-25 08:23 | bratao | File Added: network.c.v2.patch | |
| 2012-10-25 08:26 | bratao | Note Added: 0006491 | |
| 2012-10-25 09:02 | bratao | Note Added: 0006492 | |
| 2012-10-25 11:56 | LRN | Note Added: 0006493 | |
| 2012-10-25 12:53 | LRN | Note Added: 0006494 | |
| 2012-10-25 18:21 | bratao | File Added: network.c.v3.patch | |
| 2012-10-25 18:22 | bratao | Note Added: 0006496 | |
| 2012-10-25 20:47 | Christian Grothoff | Note Added: 0006497 | |
| 2012-10-26 11:13 | LRN | Note Added: 0006499 | |
| 2012-10-26 11:15 | LRN | Reproducibility | have not tried => always |
| 2012-10-26 11:15 | LRN | Status | new => resolved |
| 2012-10-26 11:15 | LRN | Resolution | open => fixed |
| 2012-10-26 11:15 | LRN | Fixed in Version | => Git master |
| 2012-10-26 11:15 | LRN | Target Version | => 0.9.4 |
| 2012-11-05 18:31 | Christian Grothoff | Fixed in Version | Git master => 0.9.4 |
| 2012-11-05 18:33 | Christian Grothoff | Status | resolved => closed |
