oneTBB icon indicating copy to clipboard operation
oneTBB copied to clipboard

eliminating global variables

Open vlastaRW opened this issue 3 years ago • 4 comments

I am redesigning my graphics editor and have been looking for library that would help with parallelization of tasks in various parts of my application. I had a brief look at TBB and unfortunately found that it makes use of global variables, which makes me feel uneasy about using it. Correct me if I am wrong.

Are there any plans to remove them and replace them with a "context" structure that would be under the control of the library user? All the provided functions would take this context as an extra parameter. You would of course keep the current functions that would use the default global "context", But if people would not be forced to rely on the global variables.


It is my humble opinion that any serious library in the 21st century should give its users full control of all its complexities and not pretend that the problem it solves is easier than it really is. Global variables are a sign of bad design (sorry for being blunt). The problems do not manifest in simple cases, for example if the application scope is known in advance and the library user can reliably guarantee that there are no conflicts. But if the software project is dynamic in nature and relies on plug-in architecture, there may be multiple independent parts of one application that want to make use of tbb and set different defaults. Or they want to delegate setting of the defaults to the application core in a controlled way - having a context object/handle instead for global variable would help in both cases. It would also make the initialization and cleanup of tbb transparent and again, give control to the library user.

I understand that it would be a major change for tbb, but I think it would be worth it.

vlastaRW avatar Aug 20 '22 10:08 vlastaRW

Hi @vlastaRW what global variables that you found in library make you uneasy? There are not so many global variables that exist in library.

pavelkumbrasev avatar Aug 22 '22 09:08 pavelkumbrasev

There are some in main.cpp, theTLS looks suspicious, I cant even find, where the create and destroy is called. Then there are things related to the threadpool. These things should be controllable by the user of the library, not handled invisibly in DllMain and other places.

vlastaRW avatar Aug 23 '22 07:08 vlastaRW

The entities that you have mentioned are parts of the internal details of the library and the lifetime of this entities is very hard question and it is also controlled by the library. As it a library details and internal properties, they should not be exposed to the user level. What do you think about it?

pavelkumbrasev avatar Aug 23 '22 10:08 pavelkumbrasev

I think that is a fundamental flaw. You are trying to make the library appear simple to its users while it is not simple. That will work in some cases, but fail in other more complex scenarios. I did some digging and found for example this: https://community.intel.com/t5/Intel-oneAPI-Threading-Building/issue-with-nested-parallel-for/td-p/818051 That seems to be one case, where different parts of an application use tbb independently and the results are problematic. My usage scenario would probably be similar and so I am concerned.

Again, let me reiterate, for simple programs, it is fine to be oblivious about the lifetime of the treadpool and possibly other stuff. For complex systems with plug-ins (think GIMP) from different authors, where various components may want to use tbb and one is called from another, the lifetime of everything must be under the control of the library user. Otherwise, unavoidable problems will arise.

My suggestion is to have two levels of API, an internal level, where the complexity of the system is not hidden, and users must manually create a threadpool and everything else that is needed for the rest to run and is currently global, and the second level, convenience API for simple scenarios (that would be the current API), which relies on global variables.

vlastaRW avatar Aug 23 '22 11:08 vlastaRW

I personally don't see any pros to do such change in the library since the lifetime of these variables controlled by system linker that should not cause any problems for end user. However, there are scenarios where you might want to control life time of thread pool to have ability to unload the library. For that task you can use task_scheduler_handle API.

pavelkumbrasev avatar Jun 21 '23 12:06 pavelkumbrasev

If there is still a concerns with library design, please, consider to reopen this issue.

pavelkumbrasev avatar Jun 21 '23 12:06 pavelkumbrasev