From cd838d3461e2ae3908a80119ef912583417d7f57 Mon Sep 17 00:00:00 2001 From: Chris Jefferson Date: Mon, 12 Dec 2022 23:31:05 +0000 Subject: [PATCH 1/2] Remove ActivateProfileColour --- doc/ref/debug.xml | 1 - lib/newprofile.g | 22 -------- src/profile.c | 136 +--------------------------------------------- 3 files changed, 3 insertions(+), 156 deletions(-) diff --git a/doc/ref/debug.xml b/doc/ref/debug.xml index 42dc6a29ef..8f21f0341f 100644 --- a/doc/ref/debug.xml +++ b/doc/ref/debug.xml @@ -839,7 +839,6 @@ Profiles are transformed into a human-readable form with <#Include Label="CoverageLineByLine"> <#Include Label="UnprofileLineByLine"> <#Include Label="UncoverageLineByLine"> -<#Include Label="ActivateProfileColour"> <#Include Label="IsLineByLineProfileActive"> diff --git a/lib/newprofile.g b/lib/newprofile.g index cabf68337f..4639d329b4 100644 --- a/lib/newprofile.g +++ b/lib/newprofile.g @@ -153,25 +153,3 @@ end); BIND_GLOBAL("UncoverageLineByLine", function() return DEACTIVATE_PROFILING(); end); - - -############################################################################# -## -## -## <#GAPDoc Label="ActivateProfileColour"> -## -## -## -## -## Called with argument true, -## -## makes GAP colour functions when printing them to show which lines -## have been executed while profiling was active via -## at any time during this GAP session. -## Passing false disables this behaviour. -## -## -## <#/GAPDoc> -BIND_GLOBAL("ActivateProfileColour",function(b) - return ACTIVATE_COLOR_PROFILING(b); -end); diff --git a/src/profile.c b/src/profile.c index 37fa85c484..28439085ca 100644 --- a/src/profile.c +++ b/src/profile.c @@ -60,11 +60,7 @@ ** We also use 1 bit to mark that we have executed this statement, so ** we can (if we want to) only output each executed statement once. ** -** We use this information in a few ways: -** -** 1) Output a straight list of every executed statement -** 2) Provide coloured printing (by wrapping PrintStatFuncs and PrintExprFuncs) -** which show which parts of an expression have been executed +** We use this information to output a straight list of every executed statement. ** ** There are not that many tricky cases here. We have to be careful that ** some Expr are integers or local variables, and not touch those. @@ -83,9 +79,8 @@ ** we provide -P (profiling) and -c (code coverage) options to the GAP ** executable so the user can start before code loading starts ** -** 3) Operating at just a line basis can sometimes be too course. We can -** see this when we use ActivateProfileColour. However, without some -** serious additional overheads, I can't see how to provide this +** 3) Operating at just a line basis can sometimes be too coarse. However, +** without some serious additional overheads, I can't see how to provide this ** functionality in output (basically we would have to store ** line and character positions for the start and end of every expression). ** @@ -135,8 +130,6 @@ static struct ProfileState int StreamWasPopened; // Are we currently outputting repeats (false=code coverage) int OutputRepeats; - // Are we colouring output (not related to profiling directly) - int ColouringOutput; // Used to generate 'X' statements, to make sure we correctly // attach each function call to the line it was executed on @@ -798,128 +791,6 @@ static Obj FuncIsLineByLineProfileActive(Obj self) } } -/**************************************************************************** -** -** We are now into the functions which deal with colouring printing output. -** This code basically wraps all the existing print functions and will colour -** their output either green or red depending on if statements are marked -** as being executed. -*/ - -static Int CurrentColour = 0; - -static void setColour(void) -{ - if(CurrentColour == 0) { - Pr("\x1b[0m", 0, 0); - } - else if(CurrentColour == 1) { - Pr("\x1b[32m", 0, 0); - } - else if(CurrentColour == 2) { - Pr("\x1b[31m", 0, 0); - } -} - -static void ProfilePrintStatPassthrough(Stat stat) -{ - Int SavedColour = CurrentColour; - if(VISITED_STAT(stat)) { - CurrentColour = 1; - } - else { - CurrentColour = 2; - } - setColour(); - OriginalPrintStatFuncsForHook[TNUM_STAT(stat)](stat); - CurrentColour = SavedColour; - setColour(); -} - -static void ProfilePrintExprPassthrough(Expr stat) -{ - Int SavedColour = -1; - // There are two cases we must pass through without touching - // From TNUM_EXPR - if(IS_REF_LVAR(stat)) { - OriginalPrintExprFuncsForHook[EXPR_REF_LVAR](stat); - } else if(IS_INTEXPR(stat)) { - OriginalPrintExprFuncsForHook[EXPR_INT](stat); - } else { - SavedColour = CurrentColour; - if(VISITED_STAT(stat)) { - CurrentColour = 1; - } - else { - CurrentColour = 2; - } - setColour(); - OriginalPrintExprFuncsForHook[TNUM_STAT(stat)](stat); - CurrentColour = SavedColour; - setColour(); - } -} - -static struct PrintHooks profilePrintHooks = - {ProfilePrintStatPassthrough, ProfilePrintExprPassthrough}; - -static Obj activate_colored_output_from_profile(void) -{ - HashLock(&profileState); - - if(profileState.ColouringOutput) { - HashUnlock(&profileState); - return Fail; - } - - ActivatePrintHooks(&profilePrintHooks); - - profileState.ColouringOutput = 1; - CurrentColour = 0; - setColour(); - - HashUnlock(&profileState); - - return True; -} - -static Obj deactivate_colored_output_from_profile(void) -{ - HashLock(&profileState); - - if(!profileState.ColouringOutput) { - HashUnlock(&profileState); - return Fail; - } - - DeactivatePrintHooks(&profilePrintHooks); - - profileState.ColouringOutput = 0; - CurrentColour = 0; - setColour(); - - HashUnlock(&profileState); - - return True; -} - -static Obj FuncACTIVATE_COLOR_PROFILING(Obj self, Obj arg) -{ - if(arg == True) - { - return activate_colored_output_from_profile(); - } - else if(arg == False) - { - return deactivate_colored_output_from_profile(); - } - else - return Fail; -} - - - - /**************************************************************************** ** *F * * * * * * * * * * * * * initialize module * * * * * * * * * * * * * * * @@ -939,7 +810,6 @@ static StructGVarFunc GVarFuncs[] = { resolution), GVAR_FUNC_0ARGS(DEACTIVATE_PROFILING), GVAR_FUNC_0ARGS(IsLineByLineProfileActive), - GVAR_FUNC_1ARGS(ACTIVATE_COLOR_PROFILING, arg), { 0, 0, 0, 0, 0 } }; From 7daffc1f00fc0cbf7c33ff6bc82e61dc1d7c02d1 Mon Sep 17 00:00:00 2001 From: Max Horn Date: Mon, 2 Jan 2023 12:49:52 +0100 Subject: [PATCH 2/2] kernel: remove unused print hooks --- src/exprs.c | 34 ++++++++++++++++++----------- src/exprs.h | 10 +-------- src/hookintrprtr.c | 54 ---------------------------------------------- src/hookintrprtr.h | 24 --------------------- src/stats.c | 35 +++++++++++++++++++----------- src/stats.h | 10 +-------- 6 files changed, 47 insertions(+), 120 deletions(-) diff --git a/src/exprs.c b/src/exprs.c index 94cab7984d..4e9c1334a5 100644 --- a/src/exprs.c +++ b/src/exprs.c @@ -1194,29 +1194,39 @@ static void RecExpr2(Obj rec, Expr expr) /**************************************************************************** ** -*F PrintExpr() . . . . . . . . . . . . . . . . . . print an expression +*V PrintExprFuncs[] . . printing function for objects of type ** -** 'PrintExpr' prints the expression . +** 'PrintExprFuncs' is the dispatching table that contains for every type of +** expressions a pointer to the printer for expressions of this type, i.e., +** the function that should be called to print expressions of this type. +*/ +static PrintExprFunc PrintExprFuncs[256]; + + +/**************************************************************************** ** -** 'PrintExpr' simply dispatches through the table 'PrintExprFuncs' to the -** appropriate printer. +*F InstallPrintExprFunc(,) */ -void PrintExpr ( - Expr expr ) +void InstallPrintExprFunc(unsigned int pos, PrintExprFunc f) { - (*PrintExprFuncs[ TNUM_EXPR(expr) ])( expr ); + GAP_ASSERT(pos < ARRAY_SIZE(PrintExprFuncs)); + PrintExprFuncs[pos] = f; } /**************************************************************************** ** -*V PrintExprFuncs[] . . printing function for objects of type +*F PrintExpr() . . . . . . . . . . . . . . . . . . print an expression ** -** 'PrintExprFuncs' is the dispatching table that contains for every type of -** expressions a pointer to the printer for expressions of this type, i.e., -** the function that should be called to print expressions of this type. +** 'PrintExpr' prints the expression . +** +** 'PrintExpr' simply dispatches through the table 'PrintExprFuncs' to the +** appropriate printer. */ -PrintExprFunc PrintExprFuncs[256]; +void PrintExpr(Expr expr) +{ + (*PrintExprFuncs[TNUM_EXPR(expr)])(expr); +} /**************************************************************************** diff --git a/src/exprs.h b/src/exprs.h index 5f0c29b113..1521970dcc 100644 --- a/src/exprs.h +++ b/src/exprs.h @@ -141,15 +141,7 @@ void PrintExpr(Expr expr); void PrintRecExpr1(Expr expr); /* needed for printing function calls with options */ -/**************************************************************************** -** -*V PrintExprFuncs[] . . printing function for objects of type -** -** 'PrintExprFuncs' is the dispatching table that contains for every type of -** expressions a pointer to the printer for expressions of this type, i.e., -** the function that should be called to print expressions of this type. -*/ -extern PrintExprFunc PrintExprFuncs[256]; +void InstallPrintExprFunc(unsigned int pos, PrintExprFunc f); /**************************************************************************** diff --git a/src/hookintrprtr.c b/src/hookintrprtr.c index 581aa178ec..901d1a45cf 100644 --- a/src/hookintrprtr.c +++ b/src/hookintrprtr.c @@ -30,9 +30,6 @@ struct InterpreterHooks * activeHooks[HookCount]; // Number of active hooks static Int HookActiveCount; -// If a print hook is current active -static Int PrintHookActive; - /**************************************************************************** ** ** Store the true values of each function we wrap for hooking. These always @@ -44,9 +41,6 @@ ExecStatFunc OriginalExecStatFuncsForHook[256]; EvalExprFunc OriginalEvalExprFuncsForHook[256]; EvalBoolFunc OriginalEvalBoolFuncsForHook[256]; -PrintStatFunc OriginalPrintStatFuncsForHook[256]; -PrintExprFunc OriginalPrintExprFuncsForHook[256]; - /**************************************************************************** ** ** These functions install implementations of eval/expr functions, @@ -83,26 +77,6 @@ void InstallExecStatFunc(Int pos, ExecStatFunc f) HashUnlock(&activeHooks); } -void InstallPrintStatFunc(Int pos, PrintStatFunc f) -{ - OriginalPrintStatFuncsForHook[pos] = f; - HashLock(&activeHooks); - if(!PrintHookActive) { - PrintStatFuncs[pos] = f; - } - HashUnlock(&activeHooks); -} - -void InstallPrintExprFunc(Int pos, PrintExprFunc f) -{ - OriginalPrintExprFuncsForHook[pos] = f; - HashLock(&activeHooks); - if(!PrintHookActive) { - PrintExprFuncs[pos] = f; - } - HashUnlock(&activeHooks); -} - static ExecStatus ProfileExecStatPassthrough(Stat stat) { GAP_HOOK_LOOP(visitStat, stat); @@ -191,34 +165,6 @@ Int DeactivateHooks(struct InterpreterHooks * hook) return 1; } -void ActivatePrintHooks(struct PrintHooks * hook) -{ - Int i; - - if (PrintHookActive) { - return; - } - PrintHookActive = 1; - for (i = 0; i < ARRAY_SIZE(ExecStatFuncs); i++) { - if (hook->printStatPassthrough) { - PrintStatFuncs[i] = hook->printStatPassthrough; - } - if (hook->printExprPassthrough) { - PrintExprFuncs[i] = hook->printExprPassthrough; - } - } -} - -void DeactivatePrintHooks(struct PrintHooks * hook) -{ - if (!PrintHookActive) { - return; - } - PrintHookActive = 0; - memcpy(PrintStatFuncs, OriginalPrintStatFuncsForHook, sizeof(PrintStatFuncs)); - memcpy(PrintExprFuncs, OriginalPrintExprFuncsForHook, sizeof(PrintExprFuncs)); -} - /**************************************************************************** ** *F * * * * * * * * * * * * * initialize module * * * * * * * * * * * * * * * diff --git a/src/hookintrprtr.h b/src/hookintrprtr.h index c0afb796f6..819b0319ff 100644 --- a/src/hookintrprtr.h +++ b/src/hookintrprtr.h @@ -19,8 +19,6 @@ void InstallEvalBoolFunc(Int, EvalBoolFunc); void InstallEvalExprFunc(Int, EvalExprFunc); void InstallExecStatFunc(Int, ExecStatFunc); -void InstallPrintStatFunc(Int, PrintStatFunc); -void InstallPrintExprFunc(Int, PrintExprFunc); /**************************************************************************** @@ -36,9 +34,6 @@ extern ExecStatFunc OriginalExecStatFuncsForHook[256]; extern EvalExprFunc OriginalEvalExprFuncsForHook[256]; extern EvalBoolFunc OriginalEvalBoolFuncsForHook[256]; -extern PrintStatFunc OriginalPrintStatFuncsForHook[256]; -extern PrintExprFunc OriginalPrintExprFuncsForHook[256]; - /**************************************************************************** ** @@ -91,25 +86,6 @@ extern struct InterpreterHooks * activeHooks[HookCount]; Int ActivateHooks(struct InterpreterHooks * hook); Int DeactivateHooks(struct InterpreterHooks * hook); -/**************************************************************************** -** -** A struct to represent the hooks allowed into printing functions -** -** -** This struct represents a list of functions which will be called whenever -** a statement or expression is printed. They can be used to provide -** simple customisation of printing. At the moment they are used by -** profiling.c, to mark statements which have been executed. -** Look at that code to get an idea how to use them. -*/ -struct PrintHooks { - void (*printStatPassthrough)(Stat stat); - void (*printExprPassthrough)(Expr stat); -}; - -void ActivatePrintHooks(struct PrintHooks * hook); -void DeactivatePrintHooks(struct PrintHooks * hook); - /**************************************************************************** ** ** We need the functions in the next three functions to be in the header, diff --git a/src/stats.c b/src/stats.c index fb69f25341..2a56a25811 100644 --- a/src/stats.c +++ b/src/stats.c @@ -1137,31 +1137,42 @@ void ClearError ( void ) } } + /**************************************************************************** ** -*F PrintStat() . . . . . . . . . . . . . . . . . . . print a statement +*V PrintStatFuncs[] . . print function for statements of type ** -** 'PrintStat' prints the statements . +** 'PrintStatFuncs' is the dispatching table that contains for every type of +** statements a pointer to the printer for statements of this type, i.e., +** the function that should be called to print statements of this type. +*/ +static PrintStatFunc PrintStatFuncs[256]; + + +/**************************************************************************** ** -** 'PrintStat' simply dispatches through the table 'PrintStatFuncs' to the -** appropriate printer. +*F InstallPrintStatFunc(,) */ -void PrintStat ( - Stat stat ) +void InstallPrintStatFunc(unsigned int pos, PrintStatFunc f) { - (*PrintStatFuncs[TNUM_STAT(stat)])( stat ); + GAP_ASSERT(pos < ARRAY_SIZE(PrintStatFuncs)); + PrintStatFuncs[pos] = f; } /**************************************************************************** ** -*V PrintStatFuncs[] . . print function for statements of type +*F PrintStat() . . . . . . . . . . . . . . . . . . . print a statement ** -** 'PrintStatFuncs' is the dispatching table that contains for every type of -** statements a pointer to the printer for statements of this type, i.e., -** the function that should be called to print statements of this type. +** 'PrintStat' prints the statements . +** +** 'PrintStat' simply dispatches through the table 'PrintStatFuncs' to the +** appropriate printer. */ -PrintStatFunc PrintStatFuncs[256]; +void PrintStat(Stat stat) +{ + (*PrintStatFuncs[TNUM_STAT(stat)])(stat); +} /**************************************************************************** diff --git a/src/stats.h b/src/stats.h index 1f783205b0..ac37b53d6a 100644 --- a/src/stats.h +++ b/src/stats.h @@ -109,15 +109,7 @@ void InterruptExecStat(void); void PrintStat(Stat stat); -/**************************************************************************** -** -*V PrintStatFuncs[] . . print function for statements of type -** -** 'PrintStatFuncs' is the dispatching table that contains for every type of -** statements a pointer to the printer for statements of this type, i.e., -** the function that should be called to print statements of this type. -*/ -extern PrintStatFunc PrintStatFuncs[256]; +void InstallPrintStatFunc(unsigned int pos, PrintStatFunc f); /****************************************************************************