Skip to content

Remove O(N^2) algorithm from repeated field extraction.#51

Merged
seancribbs merged 1 commit intomasterfrom
sdc-lists-reverse-profile
Jun 19, 2013
Merged

Remove O(N^2) algorithm from repeated field extraction.#51
seancribbs merged 1 commit intomasterfrom
sdc-lists-reverse-profile

Conversation

@seancribbs
Copy link

In messages with many values for a repeated field, lists:reverse/1 is called many times, resulting in large amounts of garbage and an unreasonable amount of time spent in decoding. This moves the list reversal until the end of the decoding, only once per repeated field.

This issue was found when a customer did a 2I query returning ~150K results. Ideally this should make it into the "0.8.1" tag and Riak 1.4, although it primarily affects the client-side.

In messages with many values for a repeated field, lists:reverse/1 is
called many times, resulting in large amounts of garbage and an
unreasonable amount of time spent in decoding. This moves the list
reversal until the end of the decoding, only once per repeated field.
@seancribbs
Copy link
Author

@beerriot
Copy link
Contributor

Confirmed that this drops a 61k-result 2i query on a 3-laptop-node cluster from 14 seconds down to half a second. There also seems to be no measurable slowdown to decoding of messages that do not have large repeated fields.

Note to other potential reviewers: just recompiling pokemon_pb is not enough - you must also recompile the beams generated from the .proto files, as the pokemon_pb code is effectively inlined in them.

Also confirmed that these tests still pass with this patch on both server and client side:

  • client_java_verify
  • client_python_verify
  • mapred_*

Looks good to me. +1

@seancribbs
Copy link
Author

Thanks @beerriot!

seancribbs pushed a commit that referenced this pull request Jun 19, 2013
Remove O(N^2) algorithm from repeated field extraction.
@seancribbs seancribbs merged commit bcde354 into master Jun 19, 2013
@seancribbs seancribbs deleted the sdc-lists-reverse-profile branch June 19, 2013 20:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants