love icon indicating copy to clipboard operation
love copied to clipboard

Expose synchronization primitives

Open DekuJuice opened this issue 3 years ago • 5 comments

Currently, the way love.thread channels do mutual exclusion is to simply grab a mutex before pushing/popping a value, and then to wait on a conditional variable if using supply/demand.

The problem with this is that the mutex goes to whichever thread grabs it first. This means it's possible for a faster running thread to continuously grab the mutex, even if another one was already blocked waiting for it to be available, resulting in starvation.

This can be worked around by using sleep or timers to slow down the thread, but that doesn't address the cause, which is that there's no apparent way to create a "fair" synchronization structures like a ticket lock using what the thread module provides.

I propose that the Mutex and Conditional objects already being used by the thread module be exposed to the user, as

  • love.thread.newMutex()
  • love.thread.newConditional()

I'm not sure if these should have equivalents to love.thread.getChannel.

This would make it more convenient to work with channels as well as other non-thread safe things like FFI pointers.

DekuJuice avatar Aug 15 '21 15:08 DekuJuice

As far as I know it's already possible to implement mutex primitives in Lua code just by using Channels, but I haven't ever found a practical use for doing that.

Can you post some sort of code example of the problem you're running into? I don't think I understand what the real (measured) problems and benefits you're alluding to are.

slime73 avatar Aug 15 '21 15:08 slime73

Unless it has some egregiously large downsides i didn't consider, one relatively simple solution to this could be to use multiple channels (including them being one-way only), and have the thread that would put stuff into them do it fairly in code (in a round-robin fashion; or some other fashion that considers "congestion" of threads).

This is actually similar to what i'm doing in a library of mine, that creates one worker thread, one named channel going to said thread, and as many (unnamed outgoing) channels as needed if the library is required from more than one thread...

though in my case, my issue was that i needed messages to go to specific threads, not that one thread's (channel's) :demand call would - if i understand you correctly - keep being blocked just because there's not enough data in the queue due to some other thread always being able to process them... sound like that's not a sync issue, but an "apparently fewer threads can already handle everything, so these other ones will idle" issue, which is to say, is not really an issue in my opinion.

zorggn avatar Aug 15 '21 16:08 zorggn

I was experimenting with having a game update in a separate thread while drawing was done in the main thread.

local game_thread_code = [[
    require("love.timer")

    local event_channel = love.thread.getChannel("event_channel")
    local draw_ready_channel = love.thread.getChannel("draw_ready_channel")
    local graphic_command_channel = love.thread.getChannel("graphic_command_channel")

    local accum = 0
    
    local mx, my = 0, 0
    
    local time = love.timer.getTime()
    local dt = 0
    
    while true do
        local events = event_channel:pop()
        if events then
            for _,e in ipairs(events) do
                if e[1] == "mousemoved" then
                    mx = e[2]
                    my = e[3]
                end
            end
        end
        
        local nt = love.timer.getTime()
        dt = nt - time
        time = nt

        if draw_ready_channel:peek() then
            graphic_command_channel:push({ mx, my })
            graphic_command_channel:push(tostring( math.floor( 1 / dt ) ) )
            draw_ready_channel:pop()
        end
        love.timer.sleep(0.001)
    end
]]

io.stdout:setvbuf("no")

require("love.timer")
require("love.event")
require("love.thread")
require("love.window")
require("love.graphics")
require("love.font")

love.window.setMode(800, 600 )

function love.run()

    local game_thread = love.thread.newThread( game_thread_code )

    local event_channel = love.thread.getChannel("event_channel")
    local draw_ready_channel = love.thread.getChannel("draw_ready_channel")
    local graphic_command_channel = love.thread.getChannel("graphic_command_channel")
    graphic_command_channel:push({ 0, 0 })
    graphic_command_channel:push("0")
    
    game_thread:start()
    
    local time = love.timer.getTime()
    local dt = 0

	-- Main loop
	return function()
        
        if not game_thread:isRunning() then error(game_thread:getError()) end

        love.event.pump()
        local events = {}
        for name,a,b,c,d,e,f in love.event.poll() do
            if name == "quit" then return 0 end
            table.insert( events, {name, a, b, c, d, e, f} )
        end        
        event_channel:push( events )
                
        local nt = love.timer.getTime()
        dt = nt - time
        time = nt                
                
        draw_ready_channel:supply("ready")
        
        if love.graphics.isActive() then
            local mpos = graphic_command_channel:demand()
            local ups = graphic_command_channel:demand()
            
            love.graphics.origin()
			love.graphics.clear(love.graphics.getBackgroundColor())
            love.graphics.print(tostring(os.clock()), 10, 10 )
            love.graphics.print(("UPS: %s"):format(ups), 10, 20 )
            love.graphics.print(("FPS: %s"):format( tostring(math.floor( 1 / dt ) )), 10, 30)
            love.graphics.circle("fill", mpos[1], mpos[2], 10)
            love.timer.sleep(0.01)
			love.graphics.present()
        end
	end
end

In this example, I want the main thread to be able to push events to the channel whenever it receives them, and the update thread to optionally pop them from the channel (so that if drawing was taking too long the game could still continue updating). When I was testing initially pushing events would randomly block for 4-5 seconds, and I presume it was because the update thread was repeatedly grabbing the mutex through pop before the main thread could take it. I can't seem to reproduce it anymore though, even after removing sleep to make the update loop run as fast as possible. While I did make some changes to the code since then, I think whether the issue occurs could be on the whim of your OS's thread scheduler.

Essentially, I want a way to enforce first in first out access to channels.

But now that I think about it, it should be possible to create some sort of ticket system using only channels. I'll do some more testing before I decide push this issue further.

DekuJuice avatar Aug 15 '21 17:08 DekuJuice

I was experimenting with having a game update in a separate thread while drawing was done in the main thread. ...

Very interesting approach, thanks for sharing.

nagolove avatar Sep 02 '21 11:09 nagolove

I don't believe it would be hard to implement a Semaphore or Lock to avoid race conditions. It looks like few syscall to make and handle a semaphore. With this primitive a lock(Mutex) can be made because it's a just a binary semaphore and it would be way easier to implement a synchronicity structure. A round robin like for example would allow you to achieve good perf by reducing thread waiting time, without causing any starvation.

alex-boulay avatar Sep 04 '21 21:09 alex-boulay

When I was testing initially pushing events would randomly block for 4-5 seconds, and I presume it was because the update thread was repeatedly grabbing the mutex through pop before the main thread could take it.

I don't think that type of problem can be caused by what you describe, although if you have something that reproduces the problem I can do some investigation.

Raw mutexes are very dangerous in a high level API because it's really easy to accidentally not handle some error condition or branch that leaves the mutex locked forever (or much longer than it should) afterward. Channel:performAtomic also exists, so I'll close this but I don't mind re-evaluating my stance if there's a compelling reason.

slime73 avatar Oct 09 '22 03:10 slime73