Skip to content

Commit

Permalink
kernel: cleanup hookintrprtr and profile code
Browse files Browse the repository at this point in the history 
- unify nameid, file, fileID, fileid -> fileid
- unify argument order: first fileid, then line
- remove unused return value from `ActivateHooks` and `DeactivateHooks`
- rename `HookCount` to `MAX_HOOK_COUNT`
- convert some `UInt` / `Int` values to just `int`
- use `BOOL`, `TRUE`, `FALSE` in more places
- change `outputStat` to take `fileid`, `line` and `type` as argument
  instead of `stat` (to enable future refactoring)
  • Loading branch information
@fingolfin
fingolfin committed Jan 11, 2023
1 parent 0eaa299 commit d1d44f2
Show file tree
Hide file tree
Showing 3 changed files with 54 additions and 61 deletions.
25 changes: 10 additions & 15 deletions src/hookintrprtr.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@


// List of active hooks
struct InterpreterHooks * activeHooks[HookCount];
struct InterpreterHooks * activeHooks[MAX_HOOK_COUNT];

// Number of active hooks
static Int HookActiveCount;
Expand Down Expand Up @@ -109,19 +109,19 @@ static Obj ProfileEvalBoolPassthrough(Expr stat)
**
*/

Int ActivateHooks(struct InterpreterHooks * hook)
void ActivateHooks(struct InterpreterHooks * hook)
{
Int i;

if (HookActiveCount == HookCount) {
return 0;
if (HookActiveCount == MAX_HOOK_COUNT) {
return;
}

HashLock(&activeHooks);
for (i = 0; i < HookCount; ++i) {
for (i = 0; i < MAX_HOOK_COUNT; ++i) {
if (activeHooks[i] == hook) {
HashUnlock(&activeHooks);
return 0;
return;
}
}

Expand All @@ -131,24 +131,20 @@ Int ActivateHooks(struct InterpreterHooks * hook)
EvalBoolFuncs[i] = ProfileEvalBoolPassthrough;
}

for (i = 0; i < HookCount; ++i) {
for (i = 0; i < MAX_HOOK_COUNT; ++i) {
if (!activeHooks[i]) {
activeHooks[i] = hook;
HookActiveCount++;
HashUnlock(&activeHooks);
return 1;
break;
}
}
HashUnlock(&activeHooks);
return 0;
}

Int DeactivateHooks(struct InterpreterHooks * hook)
void DeactivateHooks(struct InterpreterHooks * hook)
{
Int i;

HashLock(&activeHooks);
for (i = 0; i < HookCount; ++i) {
for (int i = 0; i < MAX_HOOK_COUNT; ++i) {
if (activeHooks[i] == hook) {
activeHooks[i] = 0;
HookActiveCount--;
Expand All @@ -162,7 +158,6 @@ Int DeactivateHooks(struct InterpreterHooks * hook)
}

HashUnlock(&activeHooks);
return 1;
}

/****************************************************************************
Expand Down
21 changes: 10 additions & 11 deletions src/hookintrprtr.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -70,21 +70,21 @@ extern EvalBoolFunc OriginalEvalBoolFuncsForHook[256];

struct InterpreterHooks {
void (*visitStat)(Stat stat);
void (*visitInterpretedStat)(Int line, Int pos);
void (*visitInterpretedStat)(int fileid, int line);
void (*enterFunction)(Obj func);
void (*leaveFunction)(Obj func);
void (*registerStat)(Stat stat);
void (*registerInterpretedStat)(Int line, Int pos);
void (*registerInterpretedStat)(int fileid, int line);
const char * hookName;
};


enum { HookCount = 6 };
enum { MAX_HOOK_COUNT = 6 };

extern struct InterpreterHooks * activeHooks[HookCount];
extern struct InterpreterHooks * activeHooks[MAX_HOOK_COUNT];

Int ActivateHooks(struct InterpreterHooks * hook);
Int DeactivateHooks(struct InterpreterHooks * hook);
void ActivateHooks(struct InterpreterHooks * hook);
void DeactivateHooks(struct InterpreterHooks * hook);

/****************************************************************************
**
Expand All @@ -104,9 +104,8 @@ Int DeactivateHooks(struct InterpreterHooks * hook);

#define GAP_HOOK_LOOP(member, ...) \
do { \
Int i; \
struct InterpreterHooks * hook; \
for (i = 0; i < HookCount; ++i) { \
for (int i = 0; i < MAX_HOOK_COUNT; ++i) { \
hook = activeHooks[i]; \
if (hook && hook->member) { \
(hook->member)(__VA_ARGS__); \
Expand Down Expand Up @@ -135,11 +134,11 @@ EXPORT_INLINE void RegisterStatWithHook(Stat func)
GAP_HOOK_LOOP(registerStat, func);
}

EXPORT_INLINE void InterpreterHook(Int file, Int line, Int skipped)
EXPORT_INLINE void InterpreterHook(int fileid, int line, Int skipped)
{
GAP_HOOK_LOOP(registerInterpretedStat, file, line);
GAP_HOOK_LOOP(registerInterpretedStat, fileid, line);
if (!skipped) {
GAP_HOOK_LOOP(visitInterpretedStat, file, line);
GAP_HOOK_LOOP(visitInterpretedStat, fileid, line);
}
}

Expand Down
69 changes: 34 additions & 35 deletions src/profile.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ static Obj OutputtedFilenameList;

struct StatementLocation
{
int fileID;
int fileid;
int line;
};

Expand Down Expand Up @@ -268,8 +268,8 @@ static void HookedLineOutput(Obj func, char type)
const Char *name_c = name ? CONST_CSTR_STRING(name) : "nameless";

Obj filename = GET_FILENAME_BODY(body);
UInt fileID = GET_GAPNAMEID_BODY(body);
outputFilenameIdIfRequired(fileID);
UInt fileid = GET_GAPNAMEID_BODY(body);
outputFilenameIdIfRequired(fileid);
const Char *filename_c = "<missing filename>";
if(filename != Fail && filename != NULL)
filename_c = CONST_CSTR_STRING(filename);
Expand All @@ -278,7 +278,7 @@ static void HookedLineOutput(Obj func, char type)
{
fprintf(profileState.Stream, "{\"Type\":\"X\",\"Line\":%d,\"FileId\":%d}\n",
(int)profileState.lastNotOutputted.line,
(int)profileState.lastNotOutputted.fileID);
(int)profileState.lastNotOutputted.fileid);
}

// We output 'File' here for compatibility with
Expand All @@ -288,7 +288,7 @@ static void HookedLineOutput(Obj func, char type)
"d,\"EndLine\":%d,\"File\":\"%s\","
"\"FileId\":%d}\n",
type, name_c, (int)startline, (int)endline, filename_c,
(int)fileID);
(int)fileid);
}
HashUnlock(&profileState);
}
Expand Down Expand Up @@ -427,10 +427,10 @@ static inline Int8 getTicks(void)
}


static inline void printOutput(UInt line, int nameid, int exec, int visited)
static inline void printOutput(int fileid, int line, BOOL exec, BOOL visited)
{
if (profileState.lastOutputted.line != line ||
profileState.lastOutputted.fileID != nameid ||
profileState.lastOutputted.fileid != fileid ||
profileState.lastOutputtedExec != exec) {

if (profileState.OutputRepeats) {
Expand All @@ -451,46 +451,45 @@ static inline void printOutput(UInt line, int nameid, int exec, int visited)
ticksDone = (ticks / profileState.minimumProfileTick) *
profileState.minimumProfileTick;
}
outputFilenameIdIfRequired(nameid);
outputFilenameIdIfRequired(fileid);
fprintf(profileState.Stream,
"{\"Type\":\"%c\",\"Ticks\":%d,\"Line\":%d,"
"\"FileId\":%d}\n",
exec ? 'E' : 'R', ticksDone, (int)line, (int)nameid);
exec ? 'E' : 'R', ticksDone, (int)line, (int)fileid);
profileState.lastOutputtedTime = newticks;
profileState.lastNotOutputted.line = -1;
profileState.lastOutputted.line = line;
profileState.lastOutputted.fileID = nameid;
profileState.lastOutputted.fileid = fileid;
profileState.lastOutputtedExec = exec;
}
else {
profileState.lastNotOutputted.line = line;
profileState.lastNotOutputted.fileID = nameid;
profileState.lastNotOutputted.fileid = fileid;
}
}
else {
outputFilenameIdIfRequired(nameid);
outputFilenameIdIfRequired(fileid);
fprintf(profileState.Stream,
"{\"Type\":\"%c\",\"Line\":%d,\"FileId\":%d}\n",
exec ? 'E' : 'R', (int)line, (int)nameid);
exec ? 'E' : 'R', (int)line, (int)fileid);
profileState.lastOutputted.line = line;
profileState.lastOutputted.fileID = nameid;
profileState.lastOutputted.fileid = fileid;
profileState.lastOutputtedExec = exec;
profileState.lastNotOutputted.line = -1;
}
}
}

// type : the type of the statement
// exec : are we executing this statement
// visit: Was this statement previously visited (that is, executed)
static inline void outputStat(Stat stat, int exec, int visited)
static inline void
outputStat(Int fileid, int line, int type, BOOL exec, BOOL visited)
{
UInt line;
int nameid;

// Explicitly skip these two cases, as they are often specially handled
// and also aren't really interesting statements (something else will
// be executed whenever they are).
if (TNUM_STAT(stat) == EXPR_TRUE || TNUM_STAT(stat) == EXPR_FALSE) {
if (type == EXPR_TRUE || type == EXPR_FALSE) {
return;
}

Expand All @@ -501,19 +500,17 @@ static inline void outputStat(Stat stat, int exec, int visited)
return;
}

nameid = getFilenameIdOfCurrentFunction();
outputFilenameIdIfRequired(nameid);
outputFilenameIdIfRequired(fileid);

// Statement not attached to a file
if (nameid == 0) {
if (fileid == 0) {
return;
}

line = LINE_STAT(stat);
printOutput(line, nameid, exec, visited);
printOutput(fileid, line, exec, visited);
}

static inline void outputInterpretedStat(Int file, Int line, Int exec)
static inline void outputInterpretedStat(int fileid, int line, BOOL exec)
{
CheckLeaveFunctionsAfterLongjmp();

Expand All @@ -522,14 +519,14 @@ static inline void outputInterpretedStat(Int file, Int line, Int exec)
return;
}

outputFilenameIdIfRequired(file);
outputFilenameIdIfRequired(fileid);

// Statement not attached to a file
if (file == 0) {
if (fileid == 0) {
return;
}

printOutput(line, file, exec, 0);
printOutput(fileid, line, exec, 0);
}

static void visitStat(Stat stat)
Expand All @@ -539,28 +536,29 @@ static void visitStat(Stat stat)
return;
#endif

int visited = VISITED_STAT(stat);
BOOL visited = VISITED_STAT(stat);

if (!visited) {
SET_VISITED_STAT(stat);
}

if (profileState.OutputRepeats || !visited) {
HashLock(&profileState);
outputStat(stat, 1, visited);
outputStat(getFilenameIdOfCurrentFunction(), LINE_STAT(stat),
TNUM_STAT(stat), TRUE, visited);
HashUnlock(&profileState);
}
}

static void visitInterpretedStat(Int file, Int line)
static void visitInterpretedStat(int fileid, int line)
{
#ifdef HPCGAP
if (profileState.profiledThread != TLS(threadID))
return;
#endif

HashLock(&profileState);
outputInterpretedStat(file, line, 1);
outputInterpretedStat(fileid, line, TRUE);
HashUnlock(&profileState);
}

Expand All @@ -577,16 +575,17 @@ static void registerStat(Stat stat)
{
HashLock(&profileState);
if (profileState.status == Profile_Active) {
outputStat(stat, 0, 0);
outputStat(getFilenameIdOfCurrentFunction(), LINE_STAT(stat),
TNUM_STAT(stat), FALSE, FALSE);
}
HashUnlock(&profileState);
}

static void registerInterpretedStat(Int file, Int line)
static void registerInterpretedStat(int fileid, int line)
{
HashLock(&profileState);
if (profileState.status == Profile_Active) {
outputInterpretedStat(file, line, 0);
outputInterpretedStat(fileid, line, FALSE);
}
HashUnlock(&profileState);
}
Expand Down

0 comments on commit d1d44f2

Please sign in to comment.