diff mbox series

[4/9] replay: simplify async event processing

Message ID 165062841103.526882.9023955155829649867.stgit@pasha-ThinkPad-X280
State New
Headers show
Series Record/replay refactoring and stuff | expand

Commit Message

Pavel Dovgalyuk April 22, 2022, 11:53 a.m. UTC
This patch joins replay event id and async event id into single byte in the log.
It makes processing a bit faster and log a bit smaller.

Signed-off-by: Pavel Dovgalyuk <Pavel.Dovgalyuk@ispras.ru>
---
 replay/replay-events.c   |   36 ++++++++++++++----------------------
 replay/replay-internal.h |   29 ++++++++++++++---------------
 replay/replay-snapshot.c |    1 -
 replay/replay.c          |    5 +++--
 4 files changed, 31 insertions(+), 40 deletions(-)

Comments

Richard Henderson April 26, 2022, 8:26 p.m. UTC | #1
On 4/22/22 04:53, Pavel Dovgalyuk wrote:
>   static Event *replay_read_event(void)
>   {
>       Event *event;
> +    int event_kind = replay_state.data_kind - EVENT_ASYNC;

Use the enum type.

> +/* Asynchronous events IDs */
> +
> +enum ReplayAsyncEventKind {
> +    REPLAY_ASYNC_EVENT_BH,
> +    REPLAY_ASYNC_EVENT_BH_ONESHOT,
> +    REPLAY_ASYNC_EVENT_INPUT,
> +    REPLAY_ASYNC_EVENT_INPUT_SYNC,
> +    REPLAY_ASYNC_EVENT_CHAR_READ,
> +    REPLAY_ASYNC_EVENT_BLOCK,
> +    REPLAY_ASYNC_EVENT_NET,
> +    REPLAY_ASYNC_COUNT
> +};
...
> -enum ReplayAsyncEventKind {
> -    REPLAY_ASYNC_EVENT_BH,
> -    REPLAY_ASYNC_EVENT_BH_ONESHOT,
> -    REPLAY_ASYNC_EVENT_INPUT,
> -    REPLAY_ASYNC_EVENT_INPUT_SYNC,
> -    REPLAY_ASYNC_EVENT_CHAR_READ,
> -    REPLAY_ASYNC_EVENT_BLOCK,
> -    REPLAY_ASYNC_EVENT_NET,
> -    REPLAY_ASYNC_COUNT
> -};
> -
>   typedef enum ReplayAsyncEventKind ReplayAsyncEventKind;

Merge or move the typedef with the enum definition, to keep them together.

> @@ -59,7 +59,6 @@ static const VMStateDescription vmstate_replay = {
>           VMSTATE_UINT32(has_unread_data, ReplayState),
>           VMSTATE_UINT64(file_offset, ReplayState),
>           VMSTATE_UINT64(block_request_id, ReplayState),
> -        VMSTATE_INT32(read_event_kind, ReplayState),
>           VMSTATE_UINT64(read_event_id, ReplayState),
>           VMSTATE_END_OF_LIST()
>       },

Second change here, are you bumping version at the end and I haven't seen it yet?


r~
Pavel Dovgalyuk May 4, 2022, 7:10 a.m. UTC | #2
On 26.04.2022 23:26, Richard Henderson wrote:
> On 4/22/22 04:53, Pavel Dovgalyuk wrote:
>>   static Event *replay_read_event(void)
>>   {
>>       Event *event;
>> +    int event_kind = replay_state.data_kind - EVENT_ASYNC;
> 
> Use the enum type.

Ok.

>> +/* Asynchronous events IDs */
>> +
>> +enum ReplayAsyncEventKind {
>> +    REPLAY_ASYNC_EVENT_BH,
>> +    REPLAY_ASYNC_EVENT_BH_ONESHOT,
>> +    REPLAY_ASYNC_EVENT_INPUT,
>> +    REPLAY_ASYNC_EVENT_INPUT_SYNC,
>> +    REPLAY_ASYNC_EVENT_CHAR_READ,
>> +    REPLAY_ASYNC_EVENT_BLOCK,
>> +    REPLAY_ASYNC_EVENT_NET,
>> +    REPLAY_ASYNC_COUNT
>> +};
> ...
>> -enum ReplayAsyncEventKind {
>> -    REPLAY_ASYNC_EVENT_BH,
>> -    REPLAY_ASYNC_EVENT_BH_ONESHOT,
>> -    REPLAY_ASYNC_EVENT_INPUT,
>> -    REPLAY_ASYNC_EVENT_INPUT_SYNC,
>> -    REPLAY_ASYNC_EVENT_CHAR_READ,
>> -    REPLAY_ASYNC_EVENT_BLOCK,
>> -    REPLAY_ASYNC_EVENT_NET,
>> -    REPLAY_ASYNC_COUNT
>> -};
>> -
>>   typedef enum ReplayAsyncEventKind ReplayAsyncEventKind;
> 
> Merge or move the typedef with the enum definition, to keep them together.

Ok.

>> @@ -59,7 +59,6 @@ static const VMStateDescription vmstate_replay = {
>>           VMSTATE_UINT32(has_unread_data, ReplayState),
>>           VMSTATE_UINT64(file_offset, ReplayState),
>>           VMSTATE_UINT64(block_request_id, ReplayState),
>> -        VMSTATE_INT32(read_event_kind, ReplayState),
>>           VMSTATE_UINT64(read_event_id, ReplayState),
>>           VMSTATE_END_OF_LIST()
>>       },
> 
> Second change here, are you bumping version at the end and I haven't 
> seen it yet?

I'm bumping only REPLAY_VERSION which will prevent usage of the wrong 
vmstate.
diff mbox series

Patch

diff --git a/replay/replay-events.c b/replay/replay-events.c
index db1decf9dd..7bc9d6c2ff 100644
--- a/replay/replay-events.c
+++ b/replay/replay-events.c
@@ -174,8 +174,8 @@  static void replay_save_event(Event *event)
 {
     if (replay_mode != REPLAY_MODE_PLAY) {
         /* put the event into the file */
-        replay_put_event(EVENT_ASYNC);
-        replay_put_byte(event->event_kind);
+        g_assert(event->event_kind < REPLAY_ASYNC_COUNT);
+        replay_put_event(EVENT_ASYNC + event->event_kind);
 
         /* save event-specific data */
         switch (event->event_kind) {
@@ -220,14 +220,10 @@  void replay_save_events(void)
 static Event *replay_read_event(void)
 {
     Event *event;
-    if (replay_state.read_event_kind == -1) {
-        replay_state.read_event_kind = replay_get_byte();
-        replay_state.read_event_id = -1;
-        replay_check_error();
-    }
+    int event_kind = replay_state.data_kind - EVENT_ASYNC;
 
     /* Events that has not to be in the queue */
-    switch (replay_state.read_event_kind) {
+    switch (event_kind) {
     case REPLAY_ASYNC_EVENT_BH:
     case REPLAY_ASYNC_EVENT_BH_ONESHOT:
         if (replay_state.read_event_id == -1) {
@@ -236,17 +232,17 @@  static Event *replay_read_event(void)
         break;
     case REPLAY_ASYNC_EVENT_INPUT:
         event = g_new0(Event, 1);
-        event->event_kind = replay_state.read_event_kind;
+        event->event_kind = event_kind;
         event->opaque = replay_read_input_event();
         return event;
     case REPLAY_ASYNC_EVENT_INPUT_SYNC:
         event = g_new0(Event, 1);
-        event->event_kind = replay_state.read_event_kind;
+        event->event_kind = event_kind;
         event->opaque = 0;
         return event;
     case REPLAY_ASYNC_EVENT_CHAR_READ:
         event = g_new0(Event, 1);
-        event->event_kind = replay_state.read_event_kind;
+        event->event_kind = event_kind;
         event->opaque = replay_event_char_read_load();
         return event;
     case REPLAY_ASYNC_EVENT_BLOCK:
@@ -256,18 +252,17 @@  static Event *replay_read_event(void)
         break;
     case REPLAY_ASYNC_EVENT_NET:
         event = g_new0(Event, 1);
-        event->event_kind = replay_state.read_event_kind;
+        event->event_kind = event_kind;
         event->opaque = replay_event_net_load();
         return event;
     default:
-        error_report("Unknown ID %d of replay event",
-            replay_state.read_event_kind);
+        error_report("Unknown ID %d of replay event", event_kind);
         exit(1);
         break;
     }
 
     QTAILQ_FOREACH(event, &events_list, events) {
-        if (event->event_kind == replay_state.read_event_kind
+        if (event->event_kind == event_kind
             && (replay_state.read_event_id == -1
                 || replay_state.read_event_id == event->id)) {
             break;
@@ -276,12 +271,8 @@  static Event *replay_read_event(void)
 
     if (event) {
         QTAILQ_REMOVE(&events_list, event, events);
-    } else {
-        return NULL;
     }
 
-    /* Read event-specific data */
-
     return event;
 }
 
@@ -289,13 +280,14 @@  static Event *replay_read_event(void)
 void replay_read_events(void)
 {
     g_assert(replay_mutex_locked());
-    while (replay_state.data_kind == EVENT_ASYNC) {
+    while (replay_state.data_kind >= EVENT_ASYNC
+        && replay_state.data_kind <= EVENT_ASYNC_LAST) {
         Event *event = replay_read_event();
         if (!event) {
             break;
         }
         replay_finish_event();
-        replay_state.read_event_kind = -1;
+        replay_state.read_event_id = -1;
         replay_run_event(event);
 
         g_free(event);
@@ -304,7 +296,7 @@  void replay_read_events(void)
 
 void replay_init_events(void)
 {
-    replay_state.read_event_kind = -1;
+    replay_state.read_event_id = -1;
 }
 
 void replay_finish_events(void)
diff --git a/replay/replay-internal.h b/replay/replay-internal.h
index 59797c86cf..3be2e077ad 100644
--- a/replay/replay-internal.h
+++ b/replay/replay-internal.h
@@ -12,6 +12,19 @@ 
  *
  */
 
+/* Asynchronous events IDs */
+
+enum ReplayAsyncEventKind {
+    REPLAY_ASYNC_EVENT_BH,
+    REPLAY_ASYNC_EVENT_BH_ONESHOT,
+    REPLAY_ASYNC_EVENT_INPUT,
+    REPLAY_ASYNC_EVENT_INPUT_SYNC,
+    REPLAY_ASYNC_EVENT_CHAR_READ,
+    REPLAY_ASYNC_EVENT_BLOCK,
+    REPLAY_ASYNC_EVENT_NET,
+    REPLAY_ASYNC_COUNT
+};
+
 /* Any changes to order/number of events will need to bump REPLAY_VERSION */
 enum ReplayEvents {
     /* for instruction event */
@@ -22,6 +35,7 @@  enum ReplayEvents {
     EVENT_EXCEPTION,
     /* for async events */
     EVENT_ASYNC,
+    EVENT_ASYNC_LAST = EVENT_ASYNC + REPLAY_ASYNC_COUNT - 1,
     /* for shutdown requests, range allows recovery of ShutdownCause */
     EVENT_SHUTDOWN,
     EVENT_SHUTDOWN_LAST = EVENT_SHUTDOWN + SHUTDOWN_CAUSE__MAX,
@@ -49,19 +63,6 @@  enum ReplayEvents {
     EVENT_COUNT
 };
 
-/* Asynchronous events IDs */
-
-enum ReplayAsyncEventKind {
-    REPLAY_ASYNC_EVENT_BH,
-    REPLAY_ASYNC_EVENT_BH_ONESHOT,
-    REPLAY_ASYNC_EVENT_INPUT,
-    REPLAY_ASYNC_EVENT_INPUT_SYNC,
-    REPLAY_ASYNC_EVENT_CHAR_READ,
-    REPLAY_ASYNC_EVENT_BLOCK,
-    REPLAY_ASYNC_EVENT_NET,
-    REPLAY_ASYNC_COUNT
-};
-
 typedef enum ReplayAsyncEventKind ReplayAsyncEventKind;
 
 typedef struct ReplayState {
@@ -83,8 +84,6 @@  typedef struct ReplayState {
     uint64_t block_request_id;
     /*! Prior value of the host clock */
     uint64_t host_clock_last;
-    /*! Asynchronous event type read from the log */
-    int32_t read_event_kind;
     /*! Asynchronous event id read from the log */
     uint64_t read_event_id;
 } ReplayState;
diff --git a/replay/replay-snapshot.c b/replay/replay-snapshot.c
index 7e935deb15..10a7cf7992 100644
--- a/replay/replay-snapshot.c
+++ b/replay/replay-snapshot.c
@@ -59,7 +59,6 @@  static const VMStateDescription vmstate_replay = {
         VMSTATE_UINT32(has_unread_data, ReplayState),
         VMSTATE_UINT64(file_offset, ReplayState),
         VMSTATE_UINT64(block_request_id, ReplayState),
-        VMSTATE_INT32(read_event_kind, ReplayState),
         VMSTATE_UINT64(read_event_id, ReplayState),
         VMSTATE_END_OF_LIST()
     },
diff --git a/replay/replay.c b/replay/replay.c
index cb1b68c46d..f01f79cd38 100644
--- a/replay/replay.c
+++ b/replay/replay.c
@@ -22,7 +22,7 @@ 
 
 /* Current version of the replay mechanism.
    Increase it when file format changes. */
-#define REPLAY_VERSION              0xe0200b
+#define REPLAY_VERSION              0xe0200c
 /* Size of replay log header */
 #define HEADER_SIZE                 (sizeof(uint32_t) + sizeof(uint64_t))
 
@@ -219,7 +219,8 @@  bool replay_has_event(void)
         replay_account_executed_instructions();
         res = EVENT_CHECKPOINT <= replay_state.data_kind
               && replay_state.data_kind <= EVENT_CHECKPOINT_LAST;
-        res = res || replay_state.data_kind == EVENT_ASYNC;
+        res = res || (EVENT_ASYNC <= replay_state.data_kind
+                     && replay_state.data_kind <= EVENT_ASYNC_LAST);
     }
     return res;
 }