Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Revert passive RECV to 2-tuple #12

Merged
merged 1 commit into from

2 participants

@ianbarber
Owner

OK, I think I am pretty much convinced by Gar1ts points in the other thread, some of which are cut and pasted below. This basically means the API change would just be to active, but I think that is consistent as active is fundamentally a bit different. Anyway, I'll leave the merge up to you guys - I am in favour of having passive recv in this way, but this isn't my playground :) Tests also updated.

============ C+P From Gar1t below =============

Ian, in your discussions on the flags, what was the gist of the argument that swayed the debate (in the context of recv, not in the 'zmq' messages sent in active mode)?

Looking over the examples I'm filling in (https://github.com/gar1t/zguide/tree/master/examples/Erlang), I'm not sure that API change has much value. Of the 34 calls to recv, there are three checks for rcvmore.

Even that was used more often, it doesn't really clean much up. Here are the different uses (as you well know, just getting them on paper):

    {ok, Msg, Flags} = erlzmq:recv(Socket),
    case proplist:get_bool(rcvmore, Flags) of
        true -> handle_more();
        false -> nomore()
    end

    {ok, Msg} = erlzmq:recv(Socket),
    case erlzmq:getsockopt(Socket, rcvmore) of
        {ok, 1} -> handle_more();
        {ok, 0} -> nomore()
    end

I'm not seeing the payoff here that would justify these costs:

* Big break in the API, which (at least based on what I'm seeing in the examples) would go unused most of the time
* Cost of looking up that flag on every call - this might be small, but it's there (and forced, ick)
* Potential source of fugliness and bugs (if you're tempted to match directly on the list)
@evax evax merged commit 5442d91 into zeromq:master
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Jun 10, 2011
  1. @ianbarber
This page is out of date. Refresh to see the latest.
Showing with 22 additions and 34 deletions.
  1. +15 −27 c_src/erlzmq_nif.c
  2. +2 −2 src/erlzmq.erl
  3. +5 −5 test/erlzmq_test.erl
View
42 c_src/erlzmq_nif.c
@@ -539,9 +539,6 @@ NIF(erlzmq_nif_recv)
{
erlzmq_thread_request_t req;
erlzmq_socket_t * socket;
- ERL_NIF_TERM flags_list;
- size_t value_len = sizeof(int64_t);
- int64_t flag_value = 0;
if (! enif_get_resource(env, argv[0], erlzmq_nif_resource_socket,
(void **) &socket)) {
@@ -609,26 +606,16 @@ NIF(erlzmq_nif_recv)
}
}
else {
- if(zmq_getsockopt(socket->socket_zmq, ZMQ_RCVMORE, &flag_value, &value_len)) {
- return return_zmq_errno(env, zmq_errno());
- }
enif_mutex_unlock(socket->mutex);
- // Should we send the multipart flag
- if(flag_value == 1) {
- flags_list = enif_make_list1(env, enif_make_atom(env, "rcvmore"));
- } else {
- flags_list = enif_make_list(env, 0);
- }
-
ErlNifBinary binary;
enif_alloc_binary(zmq_msg_size(&msg), &binary);
memcpy(binary.data, zmq_msg_data(&msg), zmq_msg_size(&msg));
zmq_msg_close(&msg);
- return enif_make_tuple3(env, enif_make_atom(env, "ok"),
- enif_make_binary(env, &binary), flags_list);
+ return enif_make_tuple2(env, enif_make_atom(env, "ok"),
+ enif_make_binary(env, &binary));
}
}
@@ -773,8 +760,9 @@ static void * polling_thread(void * handle)
enif_mutex_lock(r->data.recv.socket->mutex);
if (zmq_recv(r->data.recv.socket->socket_zmq, &msg,
r->data.recv.flags) ||
+ (r->data.recv.socket->active == ERLZMQ_SOCKET_ACTIVE_ON &&
zmq_getsockopt(r->data.recv.socket->socket_zmq,
- ZMQ_RCVMORE, &flag_value, &value_len) )
+ ZMQ_RCVMORE, &flag_value, &value_len)) )
{
enif_mutex_unlock(r->data.recv.socket->mutex);
if (r->data.recv.socket->active == ERLZMQ_SOCKET_ACTIVE_ON) {
@@ -802,16 +790,17 @@ static void * polling_thread(void * handle)
enif_alloc_binary(zmq_msg_size(&msg), &binary);
memcpy(binary.data, zmq_msg_data(&msg), zmq_msg_size(&msg));
zmq_msg_close(&msg);
- ERL_NIF_TERM flags_list;
-
- // Should we send the multipart flag
- if(flag_value == 1) {
- flags_list = enif_make_list1(r->data.recv.env, enif_make_atom(r->data.recv.env, "rcvmore"));
- } else {
- flags_list = enif_make_list(r->data.recv.env, 0);
- }
if (r->data.recv.socket->active == ERLZMQ_SOCKET_ACTIVE_ON) {
+ ERL_NIF_TERM flags_list;
+
+ // Should we send the multipart flag
+ if(flag_value == 1) {
+ flags_list = enif_make_list1(r->data.recv.env, enif_make_atom(r->data.recv.env, "rcvmore"));
+ } else {
+ flags_list = enif_make_list(r->data.recv.env, 0);
+ }
+
enif_send(NULL, &r->data.recv.pid, r->data.recv.env,
enif_make_tuple4(r->data.recv.env,
enif_make_atom(r->data.recv.env, "zmq"),
@@ -827,10 +816,9 @@ static void * polling_thread(void * handle)
}
else {
enif_send(NULL, &r->data.recv.pid, r->data.recv.env,
- enif_make_tuple3(r->data.recv.env,
+ enif_make_tuple2(r->data.recv.env,
enif_make_copy(r->data.recv.env, r->data.recv.ref),
- enif_make_binary(r->data.recv.env, &binary),
- flags_list));
+ enif_make_binary(r->data.recv.env, &binary)));
enif_free_env(r->data.recv.env);
enif_release_resource(r->data.recv.socket);
View
4 src/erlzmq.erl
@@ -190,8 +190,8 @@ recv({I, Socket}, Flags)
Ref when is_reference(Ref) ->
Timeout = proplists:get_value(timeout, Flags, infinity),
receive
- {Ref, Result, Flag} ->
- {ok, Result, Flag}
+ {Ref, Result} ->
+ {ok, Result}
after Timeout ->
{error, {timeout, Ref}}
end;
View
10 test/erlzmq_test.erl
@@ -15,7 +15,7 @@ hwm_test() ->
ok = hwm_loop(10, S2),
- ?assertMatch({ok, <<"test">>, _Flags}, erlzmq:recv(S1)),
+ ?assertMatch({ok, <<"test">>}, erlzmq:recv(S1)),
?assertMatch(ok, erlzmq:send(S2, <<"test">>)),
ok = erlzmq:close(S1),
ok = erlzmq:close(S2),
@@ -173,13 +173,13 @@ ping_pong({S1, S2}, Msg, active) ->
ping_pong({S1, S2}, Msg, passive) ->
ok = erlzmq:send(S1, Msg),
- ?assertMatch({ok, Msg, []}, erlzmq:recv(S2)),
+ ?assertMatch({ok, Msg}, erlzmq:recv(S2)),
ok = erlzmq:send(S2, Msg),
- ?assertMatch({ok, Msg, []}, erlzmq:recv(S1)),
+ ?assertMatch({ok, Msg}, erlzmq:recv(S1)),
ok = erlzmq:send(S1, Msg, [sndmore]),
ok = erlzmq:send(S1, Msg),
- ?assertMatch({ok, Msg, [rcvmore]}, erlzmq:recv(S2)),
- ?assertMatch({ok, Msg, []}, erlzmq:recv(S2)),
+ ?assertMatch({ok, Msg}, erlzmq:recv(S2)),
+ ?assertMatch({ok, Msg}, erlzmq:recv(S2)),
ok.
basic_tests(Transport, Type1, Type2, Mode) ->
Something went wrong with that request. Please try again.