Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

OpenMP's master, single, critical blocks #73

Closed
carstenbauer opened this issue Mar 7, 2024 · 12 comments
Closed

OpenMP's master, single, critical blocks #73

carstenbauer opened this issue Mar 7, 2024 · 12 comments

Comments

@carstenbauer
Copy link
Member

carstenbauer commented Mar 7, 2024

@vchuravy
Copy link
Member

vchuravy commented Mar 7, 2024

only the master thread runs this block, the rest skip over it

I am not sure if this is a sensible operation in Julia given tasks. The task starting the parallel for-loop is waiting on the sub-tasks and is not participating in the pfor-loop.

@carstenbauer
Copy link
Member Author

FWIW, I would probably just declare the first spawned parallel task to be the "master" of this pool of tasks. But I agree, we should think about what we can/should port and what not. I'm creating these issues just to think about this at some point :)

@vchuravy
Copy link
Member

vchuravy commented Mar 7, 2024

Also I strongly dislike the term "master" these days and try to use "primary" instead.

@vchuravy
Copy link
Member

vchuravy commented Mar 7, 2024

omp critical is a Base.Semaphore with (n=1) or just a Lock, and omp single is an inverted condition.

@MasonProtter
Copy link
Member

MasonProtter commented Mar 12, 2024

FWIW, I would probably just declare the first spawned parallel task to be the "master" of this pool of tasks. But I agree, we should think about what we can/should port and what not. I'm creating these issues just to think about this at some point :)

Alternatively, if we had a @master ... or @primary ... block or whatever we want to call it, it could be something that the spawner does after its done spawning all the worker tasks. e.g. if you wrote

@tasks for i in 1:10
    @master println("hi")
    do_stuff()
end

it could expand to something like

tasks = map(chunks(1:10; ...))
    @spawn do_stuff()
end
println("hi") # <--------- the @master code here runs before we fetch, but after we spawn
foreach(fetch, tasks)

@carstenbauer
Copy link
Member Author

These pragmas are typically used within a bigger parallel block (e.g. inside of do_stuff) to indicate that a specific subpart of it should, e.g., only be run by the master/primary thread. What you propose doesn't catch this use case because the master will never do_stuff.

@MasonProtter
Copy link
Member

I see. In that case, I dont really see any advantage over single for our use cases.

@carstenbauer
Copy link
Member Author

carstenbauer commented Mar 18, 2024

omp single is an inverted condition

@vchuravy I tried to implement this (#93) and came up with

struct SectionSingle
    first::Base.RefValue{Bool}
    lck::ReentrantLock
    SectionSingle() = new(Ref(true), ReentrantLock())
end

function try_enter(f, s::SectionSingle)
    run_f = false
    lock(s.lck) do
        if s.first[]
           run_f = true # The first task to try_enter → run f
           s.first[] = false
        end
    end
    run_f && f()
end

which can be used as

s = SectionSingle()

@tasks for i in 1:10
    @set ntasks = 10

    println(i, ": before")
    try_enter(s) do
        println(i, ": only printed by a single task")
        sleep(1)
    end
    println(i, ": after")
end

Any comments about my attempt (or better implementations)?

@vchuravy
Copy link
Member

struct SectionSingle
    first::Base.RefValue{Bool}
    lck::ReentrantLock
    SectionSingle() = new(Ref(true), ReentrantLock())
end

If you have to use a RefValue for a field it makes more sense to use a mutable struct

You also don't need a lock.

mutable struct SectionSingle
  @atomic latch::Bool
  SectionSingle() = new(false)
end


function try_enter(f, s)
    latch = @atomic :monotonic s.latch
    if latch
        return
   end
   (_, success) = @atomicreplace s.latch false => true
   if  !success
      return
   end
   f()
   return
end

@carstenbauer
Copy link
Member Author

If you have to use a RefValue for a field it makes more sense to use a mutable struct

In fact, it was a mutable struct before. Don't know why I changed it...

You also don't need a lock.

Nice, completely forgot about per-field atomics. What's the purpose of :monotonic here? Is it necessary? Is it an optimization?

@vchuravy
Copy link
Member

It is an optimization. It's a typical test-test-and-set pattern. Make a weak memory load for the original value and only do the expensive compare and swap when necessary.

@carstenbauer
Copy link
Member Author

#93

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants