[SYCL] Reduce number of getenv calls to improve performance of short-running parallel_for kernels on Windows. #4321
Conversation
Signed-off-by: rdeodhar <rajiv.deodhar@intel.com>
Signed-off-by: rdeodhar <rajiv.deodhar@intel.com>
| std::string KName = typeid(NameT *).name(); | ||
| using KI = detail::KernelInfo<KernelName>; | ||
| bool DisableRounding = | ||
| (getenv("SYCL_DISABLE_PARALLEL_FOR_RANGE_ROUNDING") != nullptr) || |
There was a problem hiding this comment.
This environment variable is documented.
I don't know if there are users of this environment variable.
Anyway, I believe, it should become deprecated first with or without any effect. It can be removed being deprecated for at least one update. @bader , could you, please, confirm it?
Also, if the reason for eliminating of this env var is reducing host overhead, one could employ SYCLConfig class from source/detail/config.hpp.
sycl/include/CL/sycl/handler.hpp
Outdated
|
|
||
| // Check whether rounding trace has been requested. | ||
| // Call getenv only once. | ||
| static bool RoundingTraceChecked = false; | ||
| static bool RoundingTrace = false; | ||
| if (!RoundingTraceChecked) { | ||
| RoundingTrace = | ||
| getenv("SYCL_PARALLEL_FOR_RANGE_ROUNDING_TRACE") != nullptr; | ||
| RoundingTraceChecked = true; | ||
| } | ||
| if (RoundingTrace) |
There was a problem hiding this comment.
This is definitely a subject of thread-unsafety pattern.
It's more convenient to use SYCLConfig class.
If needed its thread-safety can be handled in distinct patch.
There was a problem hiding this comment.
Yes, I'll switch to using the SYCLConfig infrastructure, preserve all existing env vars, and see if that is efficient enough.
There was a problem hiding this comment.
The SYCLConfig features are available from the SYCL library. They don't seem to be directly available in the headers, e.g. handler.hpp. Will need to go from header into library to use SYCLConfig.
There was a problem hiding this comment.
No change has been made in environment variables supported.
Use of getenv has been minimized, which reduces overhead when short-running kernels are executed on Windows. (Execution time of getenv appears to be greater than two orders of magnitude higher on Windows than Linux).
Thread safety is not an issue since values are only written by the processing of env var settings, and any concurrent writes will write the same values.
rdeodhar
changed the title
rdeodhar
changed the title
This change avoids repeated calls to getenv when launching parallel_for kernels.
Signed-off-by: rdeodhar rajiv.deodhar@intel.com