{"action":"created","issue":{"url":"https://api.github.com/repos/KratosMultiphysics/Kratos/issues/5033","repository_url":"https://api.github.com/repos/KratosMultiphysics/Kratos","labels_url":"https://api.github.com/repos/KratosMultiphysics/Kratos/issues/5033/labels{/name}","comments_url":"https://api.github.com/repos/KratosMultiphysics/Kratos/issues/5033/comments","events_url":"https://api.github.com/repos/KratosMultiphysics/Kratos/issues/5033/events","html_url":"https://github.com/KratosMultiphysics/Kratos/issues/5033","id":451914091,"node_id":"MDU6SXNzdWU0NTE5MTQwOTE=","number":5033,"title":"Python GIL Ownership","user":{"login":"roigcarlo","id":1935791,"node_id":"MDQ6VXNlcjE5MzU3OTE=","avatar_url":"https://avatars2.githubusercontent.com/u/1935791?v=4","gravatar_id":"","url":"https://api.github.com/users/roigcarlo","html_url":"https://github.com/roigcarlo","followers_url":"https://api.github.com/users/roigcarlo/followers","following_url":"https://api.github.com/users/roigcarlo/following{/other_user}","gists_url":"https://api.github.com/users/roigcarlo/gists{/gist_id}","starred_url":"https://api.github.com/users/roigcarlo/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/roigcarlo/subscriptions","organizations_url":"https://api.github.com/users/roigcarlo/orgs","repos_url":"https://api.github.com/users/roigcarlo/repos","events_url":"https://api.github.com/users/roigcarlo/events{/privacy}","received_events_url":"https://api.github.com/users/roigcarlo/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":7,"created_at":"2019-06-04T10:18:49Z","updated_at":"2019-06-04T16:33:10Z","closed_at":null,"author_association":"MEMBER","body":"Dear all.\r\n\r\nDuring the development of the ExaQUte project, we have been suggested by @spirali (please feel free to correct me if anything I expose here is not accurate) from IT4I to review the policy of Kratos regarding the ownership of the GIL.\r\n\r\n@RiccardoRossi and I have been looking for pros and cons about this but our conclusions are incomplete at best. \r\n\r\n# Introduction\r\n\r\n## The Gil\r\n\r\nWithout delving into details, python memory manager has problems, every object in python has an internal counter that needs to be keep track off and the process that does it is not thread safe, which means that only one thread can be in execution at the same time to maintain the coherence. Think of it as our classical problem of writing into a shared variable in parallel.\r\n\r\nThe mechanism which python uses to ensure that only one thread is in execution at the same time is called GIL (Global interpreter lock).\r\n\r\nFor more information i think a good and quick article to read is https://realpython.com/python-gil/\r\n\r\n## The performance problem\r\n\r\nAs you may imagine, this kills every possible parallelism from python and heavily damages all performance that could be achieve from running into multi-core. Luckily, since python 3 there are some notable exceptions for the GIL in which can be automatically released at certain intervals if no one is using it. This potentially allows some degree of parallelism  between computation and I/O for multi-threaded python.\r\n\r\n## Pybind11, C++, Kratos and the GIL\r\n\r\nAs you know Kratos is a code that is written mainly on C++ and makes use of python as its scripting language. To do so, we expose a series of classes and functions, some of them very CPU intensive, which can be called from python. The library used for this is called Pybind11.\r\n\r\nI think also hat is very important to remark some important points: \r\n* We explicitly prohibit calling python code from C++\r\n* Most of Kratos objects are exported to python as unique_ptr, which means that are non-copiable\r\n\r\nOnce exposing our C++ code to python, one must chose the policy for the GIL. It could be released (and re-acquired at the end of the function) or it could be kept. the default policy in python11 is to held the GIL for the functions and is what we use right now.\r\n\r\nThe main implication for this is that basically, two python threads cannot call a C++ function from Kratos at the same time.\r\n\r\nIt is also worth to mention that most of the codes that have C++ based CPU intensive tasks seem to release the GIL when calling such procedures.\r\n\r\n# Releasing the GIL\r\n\r\nSo at the sight of what I exposed ones question could be: Why do we not release the GIL for Kratos procedures? From my side I cannot answer because I exactly don't know what will happen in a couple of situations, and I would like to discuss my list of concerns.\r\n\r\nI would ask you please to consider that Kratos is running in a multi-threaded python for the situations below, even if this is not the use case you are used to. I will tackle this point later.\r\n\r\nConsider a simple multi-threaded presudo-code in C++\r\n```C++\r\n\r\nclass kratos_object {\r\npublic:\r\n    int global_var = 0;\r\n\r\n    void do_iter_ser() {\r\n        for(auto in in 1..1000) {\r\n            global_var++;\r\n        }\r\n    }\r\n\r\n    void do_iter_par() {\r\n        #pragma omp parallel for\r\n        for(auto in 1..1000) {\r\n            perform_cpu_bound_task();\r\n        }\r\n    }\r\n    \r\n    void print_me() {std::cout << \"Local var: \" << global_var << std::endl;}\r\n}\r\n\r\npybind11::module& m;\r\npy::class_<kratos_object, kratos_object* >(m,\"kratos_object\")\r\n.def(\"do_iter_ser\")\r\n.def(\"do_iter_par\")\r\n.def(\"print_me\")\r\n```\r\n\r\n## Multi-threading over non thread safe classes:\r\nNow first consider this code:\r\n\r\n```Python\r\nImport KratosMultiphysics as KM\r\n\r\ndef threaded_procedure(kratos_object):\r\n    kratos_object.do_iter_ser()\r\n\r\nmy_kratos_object = KM.create_object()\r\n\r\nthread.start_new_thread ( threaded_procedure, my_kratos_object )\r\nthread.start_new_thread ( threaded_procedure, my_kratos_object )\r\n\r\nmy_kratos_object.print_me()\r\n```\r\n\r\nWhat will print_me() print? If the GIL is held, the answer clearly is 2000, but if the GIL is released, does any mechanism ensures that there are not race conditions over `global_var`?\r\n\r\n## Multi-threading over already parallel classes:\r\nConsider now a second snippet:\r\n\r\n```Python\r\nImport KratosMultiphysics as KM\r\n\r\ndef threaded_procedure(kratos_object):\r\n    kratos_object.do_iter_par()\r\n\r\nmy_kratos_object = KM.create_object()\r\n\r\nfor _ in range(0, number_of_cores)\r\n    thread.start_new_thread ( threaded_procedure, my_kratos_object )\r\n```\r\n\r\nWhile running on a machine with 4 physical cores (OMP_NUM_THREADS=4), and being the C++ loop bound by CPU, not releasing the GIL will ensure that that task will run 4 times, with 4 threads every time.\r\n\r\nWhat would happen with the GIL released? are 16 threads trying to run at the same time? only 4 but switching context?\r\n\r\nI would like to remark that @spirali said that pinning is possible with Loom, but does this can be generalized?\r\n\r\n## Running python code in C++ classes\r\nAs we prohibit this it is not relevant, anyway, the answer is clear here. Routines with the GIL not released would not have problems, routines with the GIL released would have to temporally lock it to run the code. \r\n\r\n## Use Cases\r\n\r\nFinally, concerning the use of multithreading from Kratos. Should we allow it in the first place? I understand that this could be a main topic while speaking of libraries with Django for example, but is Kratos aiming to be run in multi-threaded python codes?\r\n\r\n# Conlusion and Opinion\r\n\r\nI hope I exposed correctly all the problems and concerns with the GIL and what is the status of Kratos today.\r\n\r\nAnswering my own questions (this is my opinion:\r\n1 - I can't see there anything that prevents race conditions while multithreading from python, and while we could take the effort to go class by class to release the GIL on those who do not have problems, in general is not worth the effort.\r\n\r\n2 - While there are platforms that would ensure correct threading pinning in CPU, i am not expert in this topic and I would not make any conclusion, but my feeling is that in general this will not be the situation.\r\n\r\n3 - Finally I will admit that I could be beneficial to be able to run in threaded environment if we aim to use kratos as a web-service for example, but I also think that there are other ways to achieve the same goal and currently I don't see any benefit on allowing or incentive this possibility. \r\n\r\nAs I said @RiccardoRossi and I would really be glad if you could bring your points of view here."},"comment":{"url":"https://api.github.com/repos/KratosMultiphysics/Kratos/issues/comments/498747264","html_url":"https://github.com/KratosMultiphysics/Kratos/issues/5033#issuecomment-498747264","issue_url":"https://api.github.com/repos/KratosMultiphysics/Kratos/issues/5033","id":498747264,"node_id":"MDEyOklzc3VlQ29tbWVudDQ5ODc0NzI2NA==","user":{"login":"Kobzol","id":4539057,"node_id":"MDQ6VXNlcjQ1MzkwNTc=","avatar_url":"https://avatars0.githubusercontent.com/u/4539057?v=4","gravatar_id":"","url":"https://api.github.com/users/Kobzol","html_url":"https://github.com/Kobzol","followers_url":"https://api.github.com/users/Kobzol/followers","following_url":"https://api.github.com/users/Kobzol/following{/other_user}","gists_url":"https://api.github.com/users/Kobzol/gists{/gist_id}","starred_url":"https://api.github.com/users/Kobzol/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/Kobzol/subscriptions","organizations_url":"https://api.github.com/users/Kobzol/orgs","repos_url":"https://api.github.com/users/Kobzol/repos","events_url":"https://api.github.com/users/Kobzol/events{/privacy}","received_events_url":"https://api.github.com/users/Kobzol/received_events","type":"User","site_admin":false},"created_at":"2019-06-04T16:33:10Z","updated_at":"2019-06-04T16:33:10Z","author_association":"NONE","body":"I think that what @spirali is trying to say is that the GIL is being misused in Kratos to provide thread-safety. While it works because of the way you use Kratos, it has several disadvantages.\r\n\r\n-  It makes Kratos dependent on a Python implementation detail. If you do not explicitly document Kratos being thread-unsafe and thus forbid users from using it from multiple threads, you rely on the fact that they will use it only with the use case that you designed - from a Python interpreter. Therefore without your own locking, you cannot use Kratos safely without Python (in a standalone scenario, from GUI without Python, in a webserver etc.). This might not be a problem for you (you should acknowledge it though).\r\n- It prevents any other Python thread from running during the execution of the Kratos function. This includes the mentioned \"concurrent.futures\", if they run inside a threadpool (or even in an event loop on a single thread), they will not be able to execute while the GIL is locked. There is indeed only one GIL per Python interpreter instance. This is also what causes problems for HyperLoom, which ideally needs to switch between Python threads/tasks during the execution of a Kratos function.\r\n\r\nAs far as I understand the pybind11 behaviour, it locks the GIL when entering C++ from Python, but it should be possible to release it during long-running C++ operations (which is what the documentation gives as an example - https://pybind11.readthedocs.io/en/master/advanced/misc.html#global-interpreter-lock-gil)."}}
{"actor":{"display_login":"Kobzol"}}