From 22b0358048d8a4fea24eb82f882d513d0bb6840a Mon Sep 17 00:00:00 2001 From: Richard Brooksby Date: Fri, 7 Sep 2012 15:46:18 +0100 Subject: [PATCH] Responding to nb's review comments. Copied from Perforce Change: 179335 ServerID: perforce.ravenbrook.com --- mps/code/event.c | 30 ++++++++++----------- mps/code/event.h | 4 +-- mps/code/eventcnv.c | 66 ++++++++++++++++++++++++++++----------------- mps/code/eventcom.h | 2 +- mps/code/eventdef.h | 12 ++++++--- mps/code/eventpro.c | 16 ++++++----- mps/code/mpsioan.c | 8 +++++- 7 files changed, 83 insertions(+), 55 deletions(-) diff --git a/mps/code/event.c b/mps/code/event.c index b3ef71fec2e..a3331d18c18 100644 --- a/mps/code/event.c +++ b/mps/code/event.c @@ -115,7 +115,7 @@ void EventSync(void) /* EventInit -- start using the event system, initialize if necessary */ -Res EventInit(void) +void EventInit(void) { /* Make local enums for all event params in order to check that the indexes in the parameter definition macros are in order, and that parameter @@ -181,8 +181,6 @@ Res EventInit(void) } else { ++eventUserCount; } - - return ResOK; } @@ -386,26 +384,26 @@ void EventDump(mps_lib_FILE *stream) #else /* EVENT, not */ -void (EventSync)(void) +void EventSync(void) { NOOP; } -Res (EventInit)(void) -{ - return ResOK; -} - - -void (EventFinish)(void) +void EventInit(void) { NOOP; } -EventControlSet (EventControl)(EventControlSet resetMask, - EventControlSet flipMask) +void EventFinish(void) +{ + NOOP; +} + + +EventControlSet EventControl(EventControlSet resetMask, + EventControlSet flipMask) { UNUSED(resetMask); UNUSED(flipMask); @@ -413,7 +411,7 @@ EventControlSet (EventControl)(EventControlSet resetMask, } -EventStringId (EventInternString)(const char *label) +EventStringId EventInternString(const char *label) { UNUSED(label); /* EventInternString is reached in varieties without events, but the result @@ -422,7 +420,7 @@ EventStringId (EventInternString)(const char *label) } -Word (EventInternGenString)(size_t len, const char *label) +Word EventInternGenString(size_t len, const char *label) { UNUSED(len); UNUSED(label); /* EventInternGenString is reached in varieties without events, but @@ -431,7 +429,7 @@ Word (EventInternGenString)(size_t len, const char *label) } -void (EventLabelAddr)(Addr addr, Word id) +void EventLabelAddr(Addr addr, Word id) { UNUSED(addr); UNUSED(id); diff --git a/mps/code/event.h b/mps/code/event.h index 81899ae34e6..590b11978ae 100644 --- a/mps/code/event.h +++ b/mps/code/event.h @@ -25,7 +25,7 @@ typedef Word EventStringId; typedef Word EventControlSet; extern void EventSync(void); -extern Res EventInit(void); +extern void EventInit(void); extern void EventFinish(void); extern EventControlSet EventControl(EventControlSet resetMask, EventControlSet flipMask); @@ -48,7 +48,7 @@ extern Word EventKindControl; /* Events are written into the buffer from the top down, so that a backtrace - can find them all starting at EventNext. */ + can find them all starting at EventLast. */ #define EVENT_BEGIN(name, structSize) \ BEGIN \ diff --git a/mps/code/eventcnv.c b/mps/code/eventcnv.c index 8e1ba9aecb2..8d22af1b898 100644 --- a/mps/code/eventcnv.c +++ b/mps/code/eventcnv.c @@ -68,7 +68,7 @@ static char *prog; /* program name */ static Bool verbose = FALSE; /* style: '\0' for human-readable, 'L' for Lisp, 'C' for CDF. */ static char style = '\0'; -static Bool reportEvents = FALSE; +static Bool reportStats = FALSE; static Bool eventEnabled[EventCodeMAX+1]; static Word bucketSize = 0; @@ -165,7 +165,7 @@ static char *parseArgs(int argc, char *argv[]) verbose = TRUE; break; case 'e': { /* event statistics */ - reportEvents = TRUE; + reportStats = TRUE; ++ i; if (i == argc) usageError(); @@ -199,9 +199,20 @@ static char *parseArgs(int argc, char *argv[]) } -/* processEvent -- process event */ +/* recordEvent -- record event + * + * This is the beginning of a system to model MPS state as events are read, + * but for the moment it just records which strings have been interned + * and which addresses have been labelled with them. + * + * NOTE: Since branch/2012-08-21/diagnostic-telemetry events are no longer + * in order in the event stream, so eventcnv would need some serious + * rethinking to model state. It's questionable that it should attempt it + * or event try to label addresses, but instead leave that to later stages of + * processing. RB 2012-09-07 + */ -static void processEvent(EventProc proc, Event event, EventClock etime) +static void recordEvent(EventProc proc, Event event, EventClock etime) { Res res; @@ -314,7 +325,7 @@ static void reportBucketResults(EventClock bucketLimit) EVENT_CLOCK_PRINT(stdout, bucketLimit); break; } - if (reportEvents) { + if (reportStats) { reportEventResults(bucketEventCount); } } @@ -331,14 +342,7 @@ static void clearBucket(void) } -/* readLog -- read and parse log - * - * This is the heart of eventcnv: It reads an event log using EventRead. - * It updates the counters. If verbose is true, it looks up the format, - * parses the arguments, and prints a representation of the event. Each - * argument is printed using printArg (see RELATION, below), except for - * some event types that are handled specially. - */ +/* printParam* -- printing functions for event parameter types */ static void printParamA(EventProc proc, char *styleConv, Addr addr) { @@ -398,6 +402,15 @@ static void printParamB(EventProc proc, char *styleConv, Bool b) } +/* readLog -- read and parse log + * + * This is the heart of eventcnv: It reads an event log using EventRead. + * It updates the counters. If verbose is true, it looks up the format, + * parses the arguments, and prints a representation of the event. Each + * argument is printed using printArg (see RELATION, below), except for + * some event types that are handled specially. + */ + static void readLog(EventProc proc) { EventCode c; @@ -405,7 +418,7 @@ static void readLog(EventProc proc) char *styleConv = NULL; /* suppress uninit warning */ /* Print event count header. */ - if (reportEvents) { + if (reportStats) { if (style == '\0') { printf(" bucket:"); for(c = 0; c <= EventCodeMAX; ++c) @@ -452,7 +465,7 @@ static void readLog(EventProc proc) bucketLimit += bucketSize; } while (eventTime >= bucketLimit); } - if (reportEvents) { + if (reportStats) { ++bucketEventCount[code]; ++totalEventCount[code]; } @@ -481,7 +494,7 @@ static void readLog(EventProc proc) break; } - switch (event->any.code) { + switch (code) { case EventLabelCode: switch (style) { @@ -567,14 +580,14 @@ static void readLog(EventProc proc) case code: \ EVENT_##name##_PARAMS(EVENT_PARAM_PRINT, name) \ break; - switch (event->any.code) { EVENT_LIST(EVENT_PRINT, X) } + switch (code) { EVENT_LIST(EVENT_PRINT, X) } } if (style == 'L') putchar(')'); putchar('\n'); fflush(stdout); } - processEvent(proc, event, eventTime); + recordEvent(proc, event, eventTime); EventDestroy(proc, event); } /* while(!feof(input)) */ @@ -582,18 +595,21 @@ static void readLog(EventProc proc) if (bucketSize != 0) { reportBucketResults(eventTime); } - if (reportEvents) { + if (reportStats) { /* report totals */ switch (style) { - case '\0': { + case '\0': printf("\n run:"); - } break; - case 'L': { + break; + case 'L': printf("(t"); - } break; - case 'C': { + break; + case 'C': + /* FIXME: This attempted to print the event stats on a row that + resembled a kind of final event, but the event clock no longer runs + monotonically upwards. */ EVENT_CLOCK_PRINT(stdout, eventTime+1); - } break; + break; } reportEventResults(totalEventCount); diff --git a/mps/code/eventcom.h b/mps/code/eventcom.h index c0eca4b7b15..c6425f6c86a 100644 --- a/mps/code/eventcom.h +++ b/mps/code/eventcom.h @@ -135,7 +135,7 @@ enum EventKindEnum { enum EventDefinitionsEnum { EVENT_LIST(EVENT_ENUM, X) - EventEnumWarningSuppressor + EventEnumWarningSuppressor /* suppress comma-at-end-of-enum warning */ }; diff --git a/mps/code/eventdef.h b/mps/code/eventdef.h index 83c7a3850f8..56d0271975b 100644 --- a/mps/code/eventdef.h +++ b/mps/code/eventdef.h @@ -30,6 +30,10 @@ * Increment the minor version when adding new events, * the median version when changing an existing event, * and the major version when changing the format of the event file. + * + * TODO: These should go into a header that appears at the start of a + * telemetry stream, but they aren't currently used. Keep updating them + * anyway. RB 2012-09-07 */ #define EVENT_VERSION_MAJOR ((unsigned)1) @@ -51,6 +55,10 @@ * in older logs, and prevents the codes being re-used. See * . * + * TODO: Rather than commenting them out, we should leave them in and mark + * them in some other way, because this header is used by event decoders and + * they still want to decode those events. RB 2012-09-07 + * * When you add an event type, you must also add an EVENT_*_PARAMS macro * specify its parameters below. * @@ -115,7 +123,7 @@ EVENT(X, ArenaExtend , 0x002f, TRUE, Arena) \ /* EVENT(X, ArenaRetract , 0x0030, TRUE, Arena) */ \ /* EVENT(X, TraceSegGreyen , 0x0031, TRUE, Seg) */ \ - /* RootScanned abuses kind, see .kind.abuse */ \ + /* RootScan abuses kind, see .kind.abuse */ \ EVENT(X, RootScan , 0x0032, TRUE, Seg) \ /* TraceStep abuses kind, see .kind.abuse */ \ /* EVENT(X, TraceStep , 0x0033, TRUE, Seg) */ \ @@ -241,8 +249,6 @@ PARAM(X, 1, P, seg) \ PARAM(X, 2, P, ss) -#define EVENT_AMCFix_PARAMS(PARAM, X) - #define EVENT_AMCFixInPlace_PARAMS(PARAM, X) #define EVENT_AMCFixForward_PARAMS(PARAM, X) \ diff --git a/mps/code/eventpro.c b/mps/code/eventpro.c index 842ee11b80d..9c8b715e5e2 100644 --- a/mps/code/eventpro.c +++ b/mps/code/eventpro.c @@ -77,9 +77,6 @@ typedef struct { #define EVENT_FORMAT_PARAM(X, index, sort, ident) #sort -#define EVENT_OFFSETS_PARAM(name, index, sort, ident) \ - offsetof(Event##name##Struct, f##index), - #define EVENT_INIT(X, name, code, always, kind) \ {#name, \ code, \ @@ -156,12 +153,13 @@ Bool EventCodeIsValid(EventCode code) static Res stringCopy(char **str_o, char *str) { char *newStr; - size_t len; + size_t len, size; len = strlen(str); - newStr = (char *)malloc(len + sizeof('\0')); + size = len + sizeof('\0'); + newStr = (char *)malloc(size); if (newStr == NULL) return ResMEMORY; - memcpy(newStr, str, len + sizeof('\0')); + memcpy(newStr, str, size); *str_o = newStr; return ResOK; } @@ -233,11 +231,14 @@ Res EventRead(Event *eventReturn, EventProc proc) Res res; EventAnyStruct anyStruct; Event event; - + + /* Read the prefix common to all event structures, in order to decode the + event size. */ res = proc->reader(proc->readerP, &anyStruct, sizeof(anyStruct)); if (res != ResOK) return res; + /* Get memory for the event. */ if (proc->cachedEvent != NULL) { event = proc->cachedEvent; proc->cachedEvent = NULL; @@ -248,6 +249,7 @@ Res EventRead(Event *eventReturn, EventProc proc) return ResMEMORY; } + /* Copy the event prefix and read the rest of the event into the memory. */ event->any = anyStruct; res = proc->reader(proc->readerP, PointerAdd(event, sizeof(anyStruct)), diff --git a/mps/code/mpsioan.c b/mps/code/mpsioan.c index ab83d5b69fc..966847575fb 100644 --- a/mps/code/mpsioan.c +++ b/mps/code/mpsioan.c @@ -16,6 +16,7 @@ #endif #include +#include #include "config.h" /* to get platform configurations */ @@ -30,11 +31,16 @@ static FILE *ioFile = NULL; mps_res_t mps_io_create(mps_io_t *mps_io_r) { FILE *f; + char *filename; if(ioFile != NULL) /* See */ return MPS_RES_LIMIT; /* Cannot currently open more than one log */ - f = fopen("mpsio.log", "wb"); + filename = getenv("MPS_TELEMETRY_FILENAME"); + if(filename == NULL) + filename = "mpsio.log"; + + f = fopen(filename, "wb"); if(f == NULL) return MPS_RES_IO;