simpler and faster message (de)serialize #21543
Conversation
JeffBezanson
added
parallel
base/distributed/messages.jl
Outdated
| end | ||
| end | ||
|
|
||
| function deserialize_msg(s::AbstractSerializer) | ||
| idx = read(s.io, UInt8) | ||
| t = msgtypes[idx] | ||
| return eval(current_module(), Expr(:body, Expr(:return, Expr(:call, deserialize_msg, QuoteNode(s), QuoteNode(t))))) | ||
| if idx == 1 |
There was a problem hiding this comment.
A comment that idx is the position of the type into the msgtypes array would be useful.
base/distributed/messages.jl
Outdated
| elseif idx == 9 | ||
| return CallMsg{:call_fetch}(deserialize(s), deserialize(s), deserialize(s)) | ||
| end | ||
| assert(false) |
There was a problem hiding this comment.
This definitely looks uglier, hence a comment stating the reason why it is being done - probably the PR description itself - would be nice.
There was a problem hiding this comment.
I could generate this code automatically, but since there are only 9 cases I wasn't sure it would actually be easier to read. I can try it that way.
base/distributed/messages.jl
Outdated
| @@ -183,7 +190,7 @@ function send_msg_(w::Worker, header, msg, now::Bool) | |||
| try | |||
| reset_state(w.w_serializer) | |||
| serialize_hdr_raw(io, header) | |||
| eval(current_module(), Expr(:body, Expr(:return, Expr(:call, serialize, QuoteNode(w.w_serializer), QuoteNode(msg))))) # io is wrapped in w_serializer | |||
| serialize_msg(w.w_serializer, msg) # io is wrapped in w_serializer | |||
There was a problem hiding this comment.
Can you be more specific?
base/distributed/messages.jl
Outdated
| end | ||
| end | ||
|
|
||
| function deserialize_msg(s::AbstractSerializer) | ||
| idx = read(s.io, UInt8) | ||
| t = msgtypes[idx] | ||
| return eval(current_module(), Expr(:body, Expr(:return, Expr(:call, deserialize_msg, QuoteNode(s), QuoteNode(t))))) |
There was a problem hiding this comment.
the outer "deserialize_msg" function will now need to be called with this "invokelatest"-style wrapper in order for (de)serialize to work (for custom added serialization functions after the event loop is initialized)
There was a problem hiding this comment.
I see, the issue is custom serialize methods added after starting workers. We need a way to do this without allocating a bunch of objects and calling eval.
|
Still working on this, and it will have to wait for #19784, but so far I have the pmap benchmark above down to allocating 41 MB. |
also separate `serialize_msg` from `serialize`
JeffBezanson
force-pushed
the
jb/messageser
branch
from
99fcff6
to
e62a7db
Compare
This fixes a couple things about message serialization.
serializewhose output could only be consumed bydeserialize_msg, so renamed toserialize_msg.Some calls toNow usesevalwere added here as part of the automatic recompilation of dependent functions #265 fix, but they don't seem necessary to me. I don't think anything adds new message types or methods to these functions, but correct me if I'm wrong.invokelatest.serialize_msganddeserialize_msgmore specialized and efficient.Benchmark:
Before:
after: