Try to improve remote bitbang speed again (#638)

* Tweak remote bitbang code.

Reducing the number of times we perform two read(2) calls in a row.

Change-Id: If8f326d4f2c4ece979af427a2213f77b9024e6dd

* Remove unnecessary block/non-block changes.

Change-Id: I095717d19483fb0daf2e6c264cab1134fd5f84cd
Signed-off-by: Tim Newsome <tim@sifive.com>

* Refactor, creating remote_bitbang_recv_buf_empty()

Change-Id: Ib678bda990b3e224abe8faec2c48401b08cf4698

* Refactor available space computation.

Creating remote_bitbang_recv_buf_contiguous_available_space() which has
a ridiculously long name but I don't see anything much better without
replacing the remote_bitbang prefix throughout the file.

Change-Id: I5d241b872a50a80a2df69fa60635207496484b7d

* Use TCP_NODELAY on remote bitbang socket.

`make` in riscv-tests/debug went from 2m40s to 1m20s.

Change-Id: I0fa9f0007774626f2306d554397877c2117e8bdb
Signed-off-by: Tim Newsome <tim@sifive.com>

* Fix build on Windows.

Change-Id: I91f3ddee794e8540c5e871ead3c09583c272bd34
Signed-off-by: Tim Newsome <tim@sifive.com>

src/jtag/drivers/remote_bitbang.c

@@ -23,6 +23,7 @@
 #ifndef _WIN32
 #include <sys/un.h>
 #include <netdb.h>
+#include <netinet/tcp.h>
 #endif
 #include "helper/system.h"
 #include "helper/replacements.h"
@@ -40,35 +41,87 @@ static uint8_t remote_bitbang_send_buf[512];
 static unsigned int remote_bitbang_send_buf_used;
 
 /* Circular buffer. When start == end, the buffer is empty. */
-static char remote_bitbang_recv_buf[64];
+static char remote_bitbang_recv_buf[256];
 static unsigned int remote_bitbang_recv_buf_start;
 static unsigned int remote_bitbang_recv_buf_end;
 
-static bool remote_bitbang_buf_full(void)
+static bool remote_bitbang_recv_buf_full(void)
 {
 	return remote_bitbang_recv_buf_end ==
 		((remote_bitbang_recv_buf_start + sizeof(remote_bitbang_recv_buf) - 1) %
 		 sizeof(remote_bitbang_recv_buf));
 }
 
-/* Read any incoming data, placing it into the buffer. */
+static bool remote_bitbang_recv_buf_empty(void)
-static int remote_bitbang_fill_buf(void)
 {
-	socket_nonblock(remote_bitbang_fd);
+	return remote_bitbang_recv_buf_start == remote_bitbang_recv_buf_end;
-	while (!remote_bitbang_buf_full()) {
+}
-		unsigned int contiguous_available_space;
+
-		if (remote_bitbang_recv_buf_end >= remote_bitbang_recv_buf_start) {
+static unsigned int remote_bitbang_recv_buf_contiguous_available_space(void)
-			contiguous_available_space = sizeof(remote_bitbang_recv_buf) -
+{
-				remote_bitbang_recv_buf_end;
+	if (remote_bitbang_recv_buf_end >= remote_bitbang_recv_buf_start) {
-			if (remote_bitbang_recv_buf_start == 0)
+		unsigned int space = sizeof(remote_bitbang_recv_buf) -
-				contiguous_available_space -= 1;
+				     remote_bitbang_recv_buf_end;
-		} else {
+		if (remote_bitbang_recv_buf_start == 0)
-			contiguous_available_space = remote_bitbang_recv_buf_start -
+			space -= 1;
-				remote_bitbang_recv_buf_end - 1;
+		return space;
+	} else {
+		return remote_bitbang_recv_buf_start -
+		       remote_bitbang_recv_buf_end - 1;
+	}
+}
+
+static int remote_bitbang_flush(void)
+{
+	if (remote_bitbang_send_buf_used <= 0)
+		return ERROR_OK;
+
+	unsigned int offset = 0;
+	while (offset < remote_bitbang_send_buf_used) {
+		ssize_t written = write_socket(remote_bitbang_fd, remote_bitbang_send_buf + offset,
+									   remote_bitbang_send_buf_used - offset);
+		if (written < 0) {
+			log_socket_error("remote_bitbang_putc");
+			remote_bitbang_send_buf_used = 0;
+			return ERROR_FAIL;
 		}
+		offset += written;
+	}
+	remote_bitbang_send_buf_used = 0;
+	return ERROR_OK;
+}
+
+typedef enum {
+	NO_BLOCK,
+	BLOCK
+} block_bool_t;
+
+/* Read any incoming data, placing it into the buffer. */
+static int remote_bitbang_fill_buf(block_bool_t block)
+{
+	if (remote_bitbang_recv_buf_empty()) {
+		/* If the buffer is empty, reset it to 0 so we get more
+		 * contiguous space. */
+		remote_bitbang_recv_buf_start = 0;
+		remote_bitbang_recv_buf_end = 0;
+	}
+
+	if (block == BLOCK) {
+		if (remote_bitbang_flush() != ERROR_OK)
+			return ERROR_FAIL;
+		socket_block(remote_bitbang_fd);
+	}
+
+	bool first = true;
+	while (!remote_bitbang_recv_buf_full()) {
+		unsigned int contiguous_available_space =
+				remote_bitbang_recv_buf_contiguous_available_space();
 		ssize_t count = read_socket(remote_bitbang_fd,
 				remote_bitbang_recv_buf + remote_bitbang_recv_buf_end,
 				contiguous_available_space);
+		if (first && block == BLOCK)
+			socket_nonblock(remote_bitbang_fd);
+		first = false;
 		if (count > 0) {
 			remote_bitbang_recv_buf_end += count;
 			if (remote_bitbang_recv_buf_end == sizeof(remote_bitbang_recv_buf))
@@ -92,26 +145,6 @@ static int remote_bitbang_fill_buf(void)
 	return ERROR_OK;
 }
 
-static int remote_bitbang_flush(void)
-{
-	if (remote_bitbang_send_buf_used <= 0)
-		return ERROR_OK;
-
-	unsigned int offset = 0;
-	while (offset < remote_bitbang_send_buf_used) {
-		ssize_t written = write_socket(remote_bitbang_fd, remote_bitbang_send_buf + offset,
-									   remote_bitbang_send_buf_used - offset);
-		if (written < 0) {
-			log_socket_error("remote_bitbang_putc");
-			remote_bitbang_send_buf_used = 0;
-			return ERROR_FAIL;
-		}
-		offset += written;
-	}
-	remote_bitbang_send_buf_used = 0;
-	return ERROR_OK;
-}
-
 typedef enum {
 	NO_FLUSH,
 	FLUSH_SEND_BUF
@@ -157,47 +190,25 @@ static bb_value_t char_to_int(int c)
 	}
 }
 
-/* Get the next read response. */
-static bb_value_t remote_bitbang_rread(void)
-{
-	if (remote_bitbang_flush() != ERROR_OK)
-		return ERROR_FAIL;
-
-	/* Enable blocking access. */
-	socket_block(remote_bitbang_fd);
-	char c;
-	ssize_t count = read_socket(remote_bitbang_fd, &c, 1);
-	if (count == 1) {
-		return char_to_int(c);
-	} else {
-		remote_bitbang_quit();
-		LOG_ERROR("read_socket: count=%d", (int) count);
-		log_socket_error("read_socket");
-		return BB_ERROR;
-	}
-}
-
 static int remote_bitbang_sample(void)
 {
-	if (remote_bitbang_fill_buf() != ERROR_OK)
+	if (remote_bitbang_fill_buf(NO_BLOCK) != ERROR_OK)
 		return ERROR_FAIL;
-	assert(!remote_bitbang_buf_full());
+	assert(!remote_bitbang_recv_buf_full());
 	return remote_bitbang_queue('R', NO_FLUSH);
 }
 
 static bb_value_t remote_bitbang_read_sample(void)
 {
-	if (remote_bitbang_recv_buf_start == remote_bitbang_recv_buf_end) {
+	if (remote_bitbang_recv_buf_empty()) {
-		if (remote_bitbang_fill_buf() != ERROR_OK)
+		if (remote_bitbang_fill_buf(BLOCK) != ERROR_OK)
-			return ERROR_FAIL;
+			return BB_ERROR;
 	}
-	if (remote_bitbang_recv_buf_start != remote_bitbang_recv_buf_end) {
+	assert(!remote_bitbang_recv_buf_empty());
-		int c = remote_bitbang_recv_buf[remote_bitbang_recv_buf_start];
+	int c = remote_bitbang_recv_buf[remote_bitbang_recv_buf_start];
-		remote_bitbang_recv_buf_start =
+	remote_bitbang_recv_buf_start =
-			(remote_bitbang_recv_buf_start + 1) % sizeof(remote_bitbang_recv_buf);
+		(remote_bitbang_recv_buf_start + 1) % sizeof(remote_bitbang_recv_buf);
-		return char_to_int(c);
+	return char_to_int(c);
-	}
-	return remote_bitbang_rread();
 }
 
 static int remote_bitbang_write(int tck, int tms, int tdi)
@@ -261,6 +272,13 @@ static int remote_bitbang_init_tcp(void)
 		close(fd);
 	}
 
+	/* We work hard to collapse the writes into the minimum number, so when
+	 * we write something we want to get it to the other end of the
+	 * connection as fast as possible. */
+	int one = 1;
+	/* On Windows optval has to be a const char *. */
+	setsockopt(fd, IPPROTO_TCP, TCP_NODELAY, (const char *) &one, sizeof(one));
+
 	freeaddrinfo(result); /* No longer needed */
 
 	if (rp == NULL) { /* No address succeeded */
@@ -314,6 +332,8 @@ static int remote_bitbang_init(void)
 	if (remote_bitbang_fd < 0)
 		return remote_bitbang_fd;
 
+	socket_nonblock(remote_bitbang_fd);
+
 	LOG_INFO("remote_bitbang driver initialized");
 	return ERROR_OK;
 }