twitter-archive / torch-ipc

A set of primitives for parallel computation in Torch
Apache License 2.0
95 stars 28 forks source link

added support for (de)serializing closures/upvalues #29

Closed nicholas-leonard closed 7 years ago

nicholas-leonard commented 7 years ago

fixes #28 for Lua 5.2. This won't work with LuaJIT (pretty sure, though untested).

nicholas-leonard commented 7 years ago

Just finished discussing this PR with @luketwitter. We think it would be best to have the user accept upvalues/closures by adding arguments to ipc.workqueue, ipc.map, and so on. We also need to add doc explaining some tricky use-cases, i.e.:

local mylib = require "mylib"

local closure = function()
   ...
   mylib.somefunction()
   ...
end

local q = ipc.workqueue()
q:write(closure, true)

In the above, mylib would get serialized. The trick would be to require the package inside the closure:

local closure = function()
   local mylib = require "mylib"
   ...
   mylib.somefunction()
   ...
end
soumith commented 7 years ago

imo closures which have upvalues should not be allowed. this makes the interface much more cleaner and usable without magic bad performance issues!

nicholas-leonard commented 7 years ago

@soumith Yeah this is why we are making it optional: workqueue.writeup allows for upvalues, workqueue.write doesn't. And all other serializations don't allow upvalues either (i.e. ipc.map, etc.).

soumith commented 7 years ago

ah cool!