[Freeciv-Dev] (PR#3649) Network usage optimization

["Arnstein Lindgard" <arnstein.lindgard@xxxxxxx>]

The attached patch reduces the total bandwidth consumption of a
typical Freeciv game with about 25%, just by editing 6 lines
of serializing code.

Enabling #define PACKET_SIZE_STATISTICS 1 reveals that
PACKET_PLAYER_INFO is responsible for about 42% of the
total bandwidth consumption in a typical Freeciv game.

Looking at the relevant serializing function, I find some apparent
inconsistencies that I don't understand. For example, you have:

struct packet_player_info {
  (...)
  int government;
  int embassy;
  (...)
};

And in the serializing (send) function:
  (...)
  dio_put_uint8(&dout, pinfo->government);
  dio_put_uint32(&dout, pinfo->embassy);
  (...)

Maybe it's been a while since somebody checked the serializing
against the actual possible sizes of the integers?

The biggest part of the package is the diplomatic states array.

Looking at the struct itself:

struct player_diplstate {
  enum diplstate_type type;     /* this player's disposition towards other */
  /* the following are for "pacts" */
  int turns_left;               /* until pact (e.g., cease-fire) ends */
  int has_reason_to_cancel;     /* 0: no, 1: this turn, 2: this or next turn */
};

And we have:

  for (i = 0; i < MAX_NUM_PLAYERS + MAX_NUM_BARBARIANS; i++) {
    dio_put_uint32(&dout, pinfo->diplstates[i].type);
    dio_put_uint32(&dout, pinfo->diplstates[i].turns_left);
    dio_put_uint32(&dout, pinfo->diplstates[i].has_reason_to_cancel);
  }

Logically speaking, the three elements do not represent more than
10 bits worth of real data, so why put 96 bits on the wire, and
multiply the difference with 34 players, for each package. 24 bits
will do.

Also, maybe it should be scalable and only communicate the diplstates
for game players, although the gain by now would be smaller. I didn't
include that in this patch.


Arnstein

--- cvs/common/packets.c        Tue Feb 18 15:52:19 2003
+++ antilag1/common/packets.c   Fri Mar  7 16:38:58 2003
@@ -875,9 +875,9 @@
 
   dio_put_uint32(&dout, pinfo->reputation);
   for (i = 0; i < MAX_NUM_PLAYERS + MAX_NUM_BARBARIANS; i++) {
-    dio_put_uint32(&dout, pinfo->diplstates[i].type);
-    dio_put_uint32(&dout, pinfo->diplstates[i].turns_left);
-    dio_put_uint32(&dout, pinfo->diplstates[i].has_reason_to_cancel);
+    dio_put_uint8(&dout, pinfo->diplstates[i].type);
+    dio_put_uint8(&dout, pinfo->diplstates[i].turns_left);
+    dio_put_uint8(&dout, pinfo->diplstates[i].has_reason_to_cancel);
   }
 
   dio_put_uint32(&dout, pinfo->gold);
@@ -932,9 +932,9 @@
 
   dio_get_uint32(&din, &pinfo->reputation);
   for (i = 0; i < MAX_NUM_PLAYERS + MAX_NUM_BARBARIANS; i++) {
-    dio_get_uint32(&din, (int *) &pinfo->diplstates[i].type);
-    dio_get_uint32(&din, &pinfo->diplstates[i].turns_left);
-    dio_get_uint32(&din, &pinfo->diplstates[i].has_reason_to_cancel);
+    dio_get_uint8(&din, (int *) &pinfo->diplstates[i].type);
+    dio_get_uint8(&din, &pinfo->diplstates[i].turns_left);
+    dio_get_uint8(&din, &pinfo->diplstates[i].has_reason_to_cancel);
   }
 
   dio_get_uint32(&din, &pinfo->gold);