Allow a list of topics to the PubSub API and batch broadcasts about phoenix_pubsub HOT 8 CLOSED

There are two places where I think the change could be made:

Phoenix.PubSub.Local

  def subscribers_with_fastlanes(pubsub_server, topic, shard) when is_atom(pubsub_server) do
    try do
      topics = List.wrap(topic)
      local = local_for_shard(shard, pubsub_server)
      # there may be a more efficient way to do this with :ets.select/2 and a match spec
      Enum.reduce(topics, [], fn(topic, subscribers) ->
        fast_concat(:ets.lookup_element(local, topic, 2), subscribers) end)
    catch
      :error, :badarg -> []
    end
  end

  defp fast_concat([], list),
    do: list
  defp fast_concat([h|t], list),
    do: fast_concat(t, [h|list])

Phoenix.PubSub.PG2Server

  def handle_info({:forward_to_local, fastlane, from_pid, pool_size, topics, msg}, name) do
    # The whole broadcast will happen inside the current process
    # but only for messages coming from the distributed system.
    for topic <- topics, do:
      Local.broadcast(fastlane, name, pool_size, from_pid, topic, msg)
    {:noreply, name}
  end

Putting the logic in Local is probably more maintainable. PG2Server is an adapter, so every new adapter will have to duplicate the code for handling lists.

from phoenix_pubsub.

This would be great to have! Found this via a related google groups discussion: https://groups.google.com/forum/#!msg/phoenix-core/0o-vNYj2Lok/gyXFHZ9iAAAJ

Since we have similar performance characteristics (although we may have many more people in a single room) I believe that this feature would help our performance.

from phoenix_pubsub.

@chrismccord @alex-shapiro I know it's been awhile since this was discussed. Anything we can do to help?

from phoenix_pubsub.

Also has anyone measured and found out this to be a performance bottleneck? From my understanding of the code, being able to batch broadcasts for multiple topics won't make a difference for local broadcasts (because we need to do a new lookup on the table for every topic) and for cases of distributed broadcasts we are only saving a single GenServer message.

So my understanding is that this change would save a couple function calls and one message, which certainly won't be the bottleneck if you are running at scale.

from phoenix_pubsub.

@josevalim I'm a little confused, in the above thread (the genesis for this ticket), you said:

Yup, just want to chime in and say it is definitely worth it perf wise.

Is that not the case then? Just curious if I'm missing something obvious 😄

from phoenix_pubsub.

I did not consider the cost of multinode communication. Thank you. :) So yes, this is worth doing but we only need to worry about it on the multinode aspect. I am assigning this to me.

from phoenix_pubsub.

Great, thank you!

from phoenix_pubsub.

This is going to be allowed in future incarnations of Phoenix.PubSub (more details about it later).

from phoenix_pubsub.