search

starpu-runtime / starpu

This is a mirror of https://gitlab.inria.fr/starpu/starpu where our development happens, but contributions are welcome here too!
https://starpu.gitlabpages.inria.fr/
GNU Lesser General Public License v2.1
58 stars 13 forks source link

Prevent codelets from being modified inside (at least) some tasks #20

Closed weslleyspereira closed 1 year ago

weslleyspereira commented 1 year ago

This PR makes a minor change that prevents codelets from being modified inside (at least) some tasks, when profiling is disabled.

With this change, codelets could be implemented as static constant expressions. Right now, as I noticed in the StarPU examples, codelets need to be "global" non-const variables. By "global", I mean: "not associated to the local scope where the task is defined".

Global variables are bad for C++ template libraries. The reason: global variables cannot be on header files, they should be either on the application executable or inside the compiled library. In the current state of StarPU, we would need to dynamically allocate space for codelets if their creation depend on template arguments. Since codelets are not that small (704B) and since dynamic memory allocation can be expensive, I would like to avoid it.

In the future, users (me included) could need both (1) profiling and (2) codelets working as constant static expressions. In this case, maybe there is a way to disable only functions reading information from per_worker_stats, which currently are starpu_codelet_display_stats() and starpu_bound_print_lp().

weslleyspereira commented 1 year ago

This issue is related to https://github.com/tlapack/tlapack/pull/211

sthibaul commented 1 year ago

Mmmm, that's right, mixing the existing profiling and the codelet profiling could be inconvenient.

Perhaps after all we should rather add a STARPU_CODELET_PROFILING environment variable. Adding the reading of the env variable can be done in _starpu_profiling_start to set a new _starpu_codelet_profiling global variable.

sthibaul commented 1 year ago

Thanks for this! I have extended it and added a test, will appear on github within a day, or you can pick it up from gitlab