computablee / DotMP

A collection of powerful abstractions for parallel programming in .NET with an OpenMP-like API.
https://computablee.github.io/DotMP/
GNU Lesser General Public License v2.1
29 stars 8 forks source link

`OpenMP.Parallel.Critical` is Inefficient #2

Closed computablee closed 1 year ago

computablee commented 1 year ago

The method OpenMP.Parallel.Critical located here is inefficient. There is a single static lock in the OpenMP.Parallel class. Consider the following OpenMP:

#pragma omp parallel
{
    #pragma omp critical
    {
        critical_func_1();
    }
    #pragma omp critical
    {
        critical_func_2();
    }
}

In this code, there are two critical regions within the parallel region, and one thread can be executing critical_func_1 while a second can be executing critical_func_2. In OpenMP.NET, this is not possible because all critical regions use a single, shared lock. Improvements to this would be appreciated.

diogolopes18-cyber commented 1 year ago

Repo seems cool, and the work you develop is pretty interesting. Would definitely like to help! My experience is mainly with ASP.NET and EF Core, however I've worked with embedded systems (C and Python) in the past

computablee commented 1 year ago

Hey @diogolopes18-cyber, sorry for the late response. Thank you! I think this issue in particular can be solved with some clever uses of locks. I haven't had the time to brainstorm a solution to this yet, but any ideas are appreciated!

diogolopes18-cyber commented 1 year ago

Yeah, I'm pretty sure I would just need to figure out a way to cleverly use the locks. Will brainstorm throughout the week and share my thoughts with you. Thanks!

computablee commented 1 year ago

@diogolopes18-cyber If we can't get it looking like the C code sample earlier, it should be fine to create instances of a OpenMP.Critical class, like so:

OpenMP.Critical critical1 = new OpenMP.Critical();
OpenMP.Critical critical2 = new OpenMP.Critical();
critical1.Critical(() => {
    critical_func_1();
});
critical2.Critical(() => {
    critical_func_2();
});
computablee commented 1 year ago

Removed "good first issue" since this appears to be a harder problem than it appeared on the surface. Brainstorming with some colleagues.

computablee commented 1 year ago

Closing this issue with a push. Figured out you can use the Action as a key to a Dictionary, and as the values store objects for locks. See https://github.com/computablee/OpenMP.NET/blob/main/OpenMP.NET/Parallel.cs#L86