On 09.11.2020 12:55, Vladimir Sementsov-Ogievskiy wrote:
06.11.2020 15:42, Andrey Shinkevich wrote:
QMP and HMP monitors read one byte at a time from the socket or stdin,
which is very inefficient. With 100+ VMs on the host, this results in
multiple extra system calls and CPU overuse.
This patch increases the amount of read data up to 4096 bytes that fits
the buffer size on the channel level.

Suggested-by: Denis V. Lunev <d...@openvz.org>
Signed-off-by: Andrey Shinkevich <andrey.shinkev...@virtuozzo.com>
---
  chardev/char-fd.c          | 64 +++++++++++++++++++++++++++++++++++++++++++++-
  chardev/char-socket.c      | 54 +++++++++++++++++++++++++++-----------
  chardev/char.c             | 40 +++++++++++++++++++++++++++++
  include/chardev/char.h     | 15 +++++++++++
  monitor/monitor.c          |  2 +-
  tests/qemu-iotests/247.out |  2 +-
  6 files changed, 159 insertions(+), 18 deletions(-)

[...]

+        ret = qio_channel_read(
+            chan, (gchar *)thl.buf, len, NULL);
+        if (ret == 0) {
+            remove_fd_in_watch(chr);
+            qemu_chr_be_event(chr, CHR_EVENT_CLOSED);
+            thl = (const JSONthrottle){0};
+            return FALSE;
+        }
+        if (ret < 0) {
+            return TRUE;
+        }

large code chunk is shared with fd_chr_read_hmp(). Would be not bad to avoid duplication..


There were two reasons to split the function:
1. Not to make the code complicated.
2. Avoid unused buffer of 4k on the stack:
   fd_chr_read_hmp() { uint8_t buf[CHR_READ_BUF_LEN];..

+        thl.load = ret;
+        thl.cursor = 0;
+    }
+
+    size = thl.load;
+    start = thl.buf + thl.cursor;

you may use uint8_t* pointer type for thl.curser and get rid of size and start variables.


For the 'start', yes. And I will want the 'size' anyway.

[...]

+int qemu_chr_end_position(const char *buf, int size, JSONthrottle *thl)
+{
+    int i;
+
+    for (i = 0; i < size; i++) {
+        switch (buf[i]) {
+        case ' ':
+        case '\n':
+        case '\r':
+            continue;
+        case '{':
+            thl->brace_count++;
+            break;
+        case '}':
+            thl->brace_count--;
+            break;
+        case '[':
+            thl->bracket_count++;
+            break;
+        case ']':
+            thl->bracket_count--;

I don't think you need to care about square brackets, as QMP queries and answers are always json objects, i.e. in pair of '{' and '}'.


I've kept the brackets because it is another condition to put a command into the requests queue (see json_message_process_token()).


Andrey

Reply via email to