Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bitstring performs faster when appended to #11

Open
bottlenecked opened this issue Aug 22, 2016 · 0 comments
Open

Bitstring performs faster when appended to #11

bottlenecked opened this issue Aug 22, 2016 · 0 comments

Comments

@bottlenecked
Copy link

bottlenecked commented Aug 22, 2016

Hi, I was going through the packer.ex code and I noticed that list serialization (same goes for maps as well) is not as fast as it could be.

According to the erlang efficiency guide bitstrings are modeled as actual byte arrays underneath that double their size once they fill up. So appending to a binary should be more efficient than inserting in the middle and reallocating+copying over to a new binary for each list item.

To try this theory I added another option when packing (append: true) and benchmarked the existing code against the new using benchee

These are the relevant code changes:

instead of reversing the list leave it as is

#old snippet:
defp do_pack_array(list, options) do
  do_pack_array(:lists.reverse(list), <<>>, options)
end
#new snippet
defp do_pack_array(list, %{append: true} = options) do
  do_pack_array(list, <<>>, options)
end

and append instead of inserting in the middle

  #old snippet
  defp do_pack_array([h|t], acc, options) do
    case do_pack(h, options) do
      { :error, _ } = error ->
        error
      binary ->
        do_pack_array(t, << binary :: binary, acc :: binary >>, options)
    end
  end
  #new snippet
  defp do_pack_array([h|t], acc, %{append: true} = options) do
    case do_pack(h, options) do
      { :error, _ } = error ->
        error
      binary ->
        do_pack_array(t, acc <> binary, options)
    end
  end

I then tested the performance using the following benchee snippet

list = Enum.to_list(1..10_000)

Benchee.run(%{time: 3}, %{
  "normal"    => fn -> MessagePack.pack!(list) end,
  "append"    => fn -> MessagePack.pack!(list, append: true) end
})

which reported the following stats


Name             ips        average    deviation         median
append        720.00      1388.89μs    (±50.43%)      1600.00μs
normal          9.77    102344.83μs    (±33.54%)     94000.00μs

Comparison:
append        720.00
normal          9.77 - 73.69x slower

Should I make a pull request for this? (I would remove the append option of course and make that the default code path)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant