On 07/14/2011 11:55 AM, Michal Novotny wrote:
+/* Time measuring facility */
+extern int time_measurement_type;
+extern uint64_t time_saveram1;
+extern uint64_t time_saveram2;
+extern uint64_t time_saveram3;
+extern uint64_t time_savedisk1;
+extern uint64_t time_savedisk2;
+extern uint64_t time_savedisk3;
+extern uint64_t time_savetotal1;
+extern uint64_t time_savetotal2;
+extern uint64_t time_savetotal3;
Arrays, please. :) Or perhaps you can store this information directly
into a QDict. I'm not sure, read until the end.
+extern uint64_t time_savewait_input0;
+extern uint64_t time_save_measured;
+extern uint64_t time_loadpart1;
+extern uint64_t time_loadpart2;
+extern uint64_t time_loadpart3;
+extern uint64_t time_loadpart4;
+extern uint64_t time_load_measured;
+
+#define TIME_MEASUREMENT_NONE 0x00
+#define TIME_MEASUREMENT_SAVE 0x01
+#define TIME_MEASUREMENT_LOAD 0x02
+#define TIME_GET(type, name, stage) time_##type##name##stage
+#define TIME_SET(type, name, stage, tv) time_##type##name##stage = tv
+#define TIME_ADD(type, name, stage, tv) time_##type##name##stage += tv
This forces type/name/stage to be fixed, so it is worse than before; at
this point you could remove the macros altogether.
What I disliked in v1 was:
1) this part (duplicated twice, in time_set and time_add)
if (strcmp(name, "ram") == 0)
time_set_ram(stage, tv);
if (strcmp(name, "disk") == 0)
time_set_disk(stage, tv);
if (strcmp(name, "wait-input") == 0)
time_save_wait_input = tv;
if (strcmp(name, "total") == 0)
time_set_total(stage, tv);
2) even more shotgun cut-and-paste in the small functions
+static void time_set_ram(int stage, uint64_t tv)
+{
+ if ((stage < 0) || (stage > 3))
+ return;
+
+ time_save_ram[stage - 1] = tv;
+}
+
+static void time_set_disk(int stage, uint64_t tv)
+{
+ if ((stage < 0) || (stage > 3))
+ return;
+
+ time_save_disk[stage - 1] = tv;
+}
+
etc. (By the way, the test on stage should be <= 0 for all of them!).
3) the fact that you put this in vl.c
Whenever you have duplication like that, you probably should use a more
generic data structure, or not use any data structure at all (just
variables). In the latter case, no funky macros or nothing---just
variables.
My first thought was to inline everything, but given how v2 looks like,
perhaps the abstraction was actually good to have---just the
implementation was ugly---and my judgement was wrong.
Perhaps you can store everything from the beginning in the QDict that
you will use for the monitor command?
Paolo
