do not use static variables in CCTK_DECLARE_ARGUMENTS

[Roland Haas]

Currently CCTK_DECLARE_VARIABLES (schematically) looks like this:

static int cctk_varFooIdx = -1
CCTK_REAL * Foo;
if(cctk_varFooIdx == -1) cctk_varFooIdx = CCTK_VarIndex("BAR::FOO");
Foo = CCTK_VarDataPtrI(cctkGH, cctk_varFooIdx, 0)

ie for each Cactus variable there is a static integer variable that is initialized with the grid variable index the first time the function is called. This way Cactus avoids having many (expensive) calls to CCTK_VarIndex for each scheduled routine. However the static variables are undesirable for at least two reasons: (a) they make it impossible to call scheduled functions in a multi-threaded way (or DECLARE_CCTK_ARGUMENTS from within an #pragma omp parallel section) and (b) there is one such variable and one if statement per grid variable potentially being slow for thorns with many variables (eg GRHydro).

The pull request https://bitbucket.org/cactuscode/cactus/pull-requests/81/cactus-remove-static-variables-in/diff removes the static variables in scheduled functions in favor for a set of global variables (of similar name) that are initialized once when Cactus starts. Fortran scheduled functions do similar (worse actually since they do contain calls taking strings as arguments) in their C-to-Fortran wrappers, though the current pull request does not address them yet.

The pull request adds a new (auto-generated) function to each thorn that is called when a thorn is enabled to set up the thorn's variables. This needs to be a separate function and cannot be done in the existing function CactusBindingsVariables_${thorn}_Initialise() due to issues with how Cactus handles public variables of implementations if I remember correctly.

[Roland Haas]

Please review.

[Erik Schnetter]

A belated comment on the implementation: There could be global variables, one per grid function, that are initialized to -1, and which are set by the flesh for all active thorns. This way not extra functions need to be scheduled, and thorns can use these global variables to look up variable indices.

[Roland Haas]

No worries. I had not properly tagged the ticket anyway and this is not something to go in before the 2020_05 release anyway.

The code basically already uses global variables but one per thorn*variable_it_has_access to. The global variables are the various cctki_vi_CCTK_THORN_structstructs. At startup time the flesh sets up the binding by initializing the variables in each struct (one per variable accessible to the thorn). Since this needs to be done in generated code (to get at the names of the global variables / structs) I cannot write a fixed function for the flesh. There is no new scheduled function, however each thorn now has one more auto-generated function CactusBindingsVarIndex_${thorn}_Initialise which the flesh calls these after it has called all CactusBindingsVariables_${thorn}_Initialise functions. The need to have a separate set of functions comes from the fact that, for public or protected variables, each thorn that inherits from an implementation providing them declares its own global variable for those inherited variables. Since an inheriting thorn may activate itself before the implementation it inherits from I must wait until all thorns are activated.

Lookup is then:

CCTK_JOIN_TOKENS(cctki_vi_, CCTK_THORN).$varname0)

in https://bitbucket.org/cactuscode/cactus/pull-requests/81/cactus-remove-static-variables-in/diff#Llib/sbin/GridFuncStuff.plT622 .

If I want to reduce the number of global variables, I would have to keep track of the type of each variable (PRIVATE or PUBLIC / PROTECTED) and use separate set of variables for PRIVATE (one per thorn) and PUBLIC / PROTECTED (one per implementation). Certainly doable (the information is present in GridFunStuff to eg create DECLARE_CCTK_ARGUMENTS which needs to make the same distinction) but a bit more work for the benefit of having fewer variables (in total, not per thorn). This would make for fewer global variables (counting each struct member as a variable).

Was that your concern?

[Erik Schnetter]

My main concern was that a single autogenerated loop for the “flesh” (i.e. in the cctk bindings) would be easier to set up than a mechanism with various scheduled functions. I don’t have any functional concern.

-erik

[Roland Haas]

There are no extra scheduled functions in the Cactus sense. I generate one helper function per thorn that is directly called by the flesh, mostly because this is the way grid variable registration is done as well (one function CactusBindingsVariables_${thorn}_Initialise per thorn to register its grid variables with the flesh) so having a CactusBindingsVarIndex_${thorn}_Initialise(void) seemed most natural.

[Roland Haas]

Unless objected I will apply this after 2020-06-01.

[Roland Haas]

Applied as git hash 7628573b "Cactus: remove static variables in DECLARE_CCTK_ARGUMENTS" of cactus