Turn O(n^2) reverse into O(n)

https://github.com/nominolo/HTTP/commit/b9bd0a08fa09c6403f91422e3b23f08d339612eb

by _ks3e • 13 years ago

175 90 13 years ago

github.com

seliopou3• 13 years ago

I for one prefer the more "algebraic" implementation. Why worry about performance when you get theorems for free?!

Monads.

dinkumthinkum• 13 years ago

Your users might care ...

100982• 13 years ago

http://imgur.com/QIVtdxM.jpg

oakwhiz• 13 years ago

Why not upgrade the compiler to detect the algebraic implementation and optimize it to the buffer implementation?

cliffbean2• 13 years ago

Because if it's a common pattern, then it's nice to put it in a common library anyway, instead of having everyone write it out by hand. It makes the code cleaner and more efficient. Win win!

tikhonj2• 13 years ago

It actually does this in a bunch of cases; see Haskell's stream fusion[1] and rewrite rules in general[2] for examples.

However, there aren't any rewrite rules for list-based code, at least with the standard library. Rewrite rules are generally used for libraries designed explicitly with performance in mind like Vector and Bytestring; the standard String type and the Prelude don't fall into this category.

[1] http://stackoverflow.com/questions/578063/what-is-haskells-s...

[2] http://www.haskell.org/ghc/docs/7.0.1/html/users_guide/rewri...

kenko• 13 years ago

I for one prefer the (appropriately) unthinking anti-intellectualism.

Hacker News.

coherentpony• 13 years ago

I think (s)he was being sarcastic.

tibbon• 13 years ago

My Haskell skills still are growing. Can someone explain?

seliopou3• 13 years ago

The original line was something like this:

  acc = "";
  for (var i = 0; i < strings.length; i++) {
      acc += strings[i]
  }

The new version is something like this:

  strings.join("")

The second version can pre-allocate a string and copy characters into that string under the hood. The first version has to allocate a new string and recopy the characters on every iteration.

fekberg3• 13 years ago

I recall when I studied algorithms and data structures, but just swapping to using references/pointers instead of copying memory, I could speed up my algorithms ridiculously. Correct me if I am wrong. "Small" things like these (as we just saw) can be really beneficial!

dmak• 13 years ago

How is the original (the for loop that you mentioned) O(n^2)? Isn't that O(n)? Isn't acc += strings[i] a O(1) operation?

tmoertel2• 13 years ago

Even if the buffers were vanilla Strings, and there were no pre-allocation, the patch would still have fixed the quadratic-runtime problem. Consider that, for vanilla Strings, the buffer ops are implemented like so:

    buf_append bufOps = (++)
    buf_concat bufOps = concat

Since (++) has a running time that's linear in the length of its left argument, you want to treat it as a right-associative operator to prevent quadratic runtime when joining a bunch of strings.

But, going back to the patch in question, the original code was effectively this:

    foldr (flip (++)) [] strs

Since flip had been applied to (++), the resulting operator you now want to treat as left associative to avoid quadratic blow-up. The code, however, uses the right-associative fold, foldr, to join the list of strings str.

The patch fixes this problem by replacing the code with the equivalent code

    concat (reverse strs)

And how is concat defined in the libraries? Looking at the source [1], it's

    concat = foldr (++) []

Thus the fix is basically

    foldr (++) [] (reverse strs).

This version reverses strs, at linear-time cost, to be able to apply the normal, unflipped (++) with a right-associative fold and thus avoid the dreaded quadratic blow-up.

[1] http://hackage.haskell.org/package/base-4.6.0.1/docs/src/GHC...

EDITED TO ADD: Also, if you knew the buffers were vanilla Strings, you could even eliminate the reverse overhead since

    foldr op z xs == foldl (flip op) z (reverse xs)

for all finite lists xs. Thus you could flip (++) and use foldl instead of foldr to get an efficient implementation like this:

    foldl (flip (++)) [] strs

auggierose2• 13 years ago

It is a standard mistake that people sometimes make when working with lists. Lists have a O(1) complexity of prepending an element, and O(n) complexity of appending an element. So when you have to append a bunch of elements to a list, it is better to reverse the list, prepend the elements, and then reverse the list again, instead of just appending all elements. The former approach has linear complexity, the latter quadratic.

mhogomchungu• 13 years ago

A list manager with a pointer to the last element on the list should also be able to append to the managed list in O(1)

aidenn02• 13 years ago

Lists don't have O(n) complexity of appending an element, if you keep track of the tail element.

thinkpad204• 13 years ago

I'd definitely like to see an explanation of what's going on, both at a high level and the specifics of the code. As a Haskell beginner-intermediate, I don't really know what most of those functions are doing (much less the context of what that function's purpose is), but I feel I could probably understand an explanation if it were given.

MBCook3• 13 years ago

I think I know enough Haskell to explain.

Old version:

    foldr (flip (buf_append bufOps)) (buf_empty bufOps) strs

foldr is a combination map/reduce in one. It takes a value (the accumulator), runs a function on the rightmost value in a list called str (that's why it's 'foldr'), and combines the two together into a new accumulator value.

Let's say we have a list of numbers, the addition operator, and an accumulator of 0.

    foldr op acc list
    foldr + 0 [1, 2, 3, 4]
    a = 0, list = [1, 2, 3, 4]
    a = 0 + 4, list = [1, 2, 3]
    a = 4 + 3, list = [1, 2]
    a = 7 + 1, list = [1]
    a = 8, list = []
    Result is 8

One thing to note is that this walks the list 'backward', and in Haskell it's a singly linked list. That means that each time it has to walk the full list. I believe this is why it's O(n^2).

It looks like the accumulator starts with (buf_empty bufOps), which from context I assume is an empty buffer structure. The operator is (flip (but_append bufOps)), which looks like it adds values onto the bufOps structure when called. Each time it goes through this process, it appends one element.

Because they're using foldr and the list is walked 'backward', this has the natural side effect that the order of the items in strs is reversed as it's added to bufOps.

New version:

    buf_concat bufOps $ reverse strs

The $ is a way of controlling precedence in Haskell, you can replace it with parenthesis. So we get:

    buf_concat bufOps ( reverse strs )

So reverse just reverses the order of a list, getting things in the order the code wants them. Then it calls buf_concat with the current structure and the items it wants added. So instead of taking one existing list and adding 6 new elements at the end one at a time, it takes one existing list and adds a list of 6 elements to the end once.

It seems like a relatively simple change. I wonder if using foldl or foldl' to avoid having to rewalk the list every time would have performance similar to the new line.

I find the new line simpler and cleaner either way, but was the choice of a right-fold the cause of the performance problem?

PKop• 13 years ago

Just so no one reading is confused, should be: Result is 10 in foldr example. Just forgot one element from list.

quchen2• 13 years ago

One thing to note is that this walks the list 'backward'

No, foldr walks through the list forward, and exactly once. Your example is evaluated as

    foldr (+) 0 [1, 2, 3, 4]
    = 1 + foldr (+) 0 [2,3,4]
    = 1 + (2 + foldr (+) 0 [3,4])
    = 1 + (2 + (3 + foldr (+) 0 [4]))
    = 1 + (2 + (3 + (4 + foldr (+) 0 [])))
    = 1 + (2 + (3 + (4 + 0)))

peteretep• 13 years ago

    $ is a way of controlling precedence in Haskell

But much more importantly, is `apply`.

anonymouscowar1• 13 years ago

Do people try and optimize Haskell programs? This is a part of Haskell that terrifies me.

fekberg3• 13 years ago

Shouldn't we optimize all our programs?

eru6• 13 years ago

Only the programs that aren't good (eg fast, simple, small) enough.

cliffbean2• 13 years ago

Sure they do! Optimizing is more than just low-level efficiency. It's also about using appropriate data structures and algorithms, and using advantageous APIs (like buf_concat here) when they're applicable.

seliopou3• 13 years ago

Look. I love Haskell. I usually formulate solutions to programming problems in my head in Haskell before I write code in whatever language I have to deal with. Constructing programs using combinators? Great. Assuming referential transparency everywhere? Great. It's a great language.

But there comes a point in time where you have to project that stuff all away and get down to the brass tacks of what's happening in your system, leaving behind the Platonic heaven you've constructed for yourself. As the patch proves without a doubt, this is very possible to do in Haskell. But sadly, the general ethos of the Haskell community does not bend in that direction. I am a lover of the language, but at the same time a vocal critic of the community in that respect.

eru6• 13 years ago

Why? There are lots of people in the Haskell community interested in speed.

tikhonj2• 13 years ago

Given the amount of effort and mindshare that libraries like Vector, Bytestring and Repa get in the Haskell community, I really don't see where you got your impression. Not everyone cares about performance, sure, but plenty of people do leading to some heavily optimized libraries.

In this day and age, having everyone worry about performance is patently absurd. Most programs don't need to be heavily optimized, so prioritizing programmer efficiency and correctness just makes sense. And Haskell does have the libraries, language features and people to optimize the parts that need optimization!

freyrs3• 13 years ago

Why is Haskell different than any other high level language in this regard? All high level languages make compromises between expressivity and performance. If you want to code really close to the metal, C exists and you know where to find it.

tmoertel2• 13 years ago

Yes! Haskell is a delight for optimization because you can use algebraic reasoning and calculation to derive more efficient code. For example, if I had the following code:

    (concat . reverse . map reverse) strs

I could easily replace it with the following code, which is simpler, faster, and reduces memory churn:

    (reverse . concat) strs

You can prove that these two implementations produce the same results (see [1] for a step-by-step proof). After you do the proofs for a while, you'll start seeing similar optimizations everywhere – and claiming them.

[1] https://gist.github.com/tmoertel/7968466

xxpor• 13 years ago

If you can derive this and prove it, shouldn't this optimization be done (theoretically) completely automatically by the compiler?

aidenn02• 13 years ago

This looks like a case of someone using a profiler to optimize a haskell program. A profiler is nearly always the best place to start when optimizing a program, regardless of the language.

jberryman• 13 years ago

Here is the context

http://www.reddit.com/r/haskell/comments/1sh67u/the_reason_w...

djulius2• 13 years ago

I guess nobody ever got upvoted for using StringBuffer/StringBuilder instead of String concatenation in Java.

m0shen• 13 years ago

In JDK 7 '+' concatenation actually uses StringBuilder under the hood, so probably nobody cares.

I can't tell if you're being facetious.

djulius2• 13 years ago

Indeed I was.

It's a good safety barrier against hype things.

drostie• 13 years ago

Explanation:

bufOps is a dictionary which holds a bunch of functions accessed with the getters on it. For the sake of this comment, we can concretize and use (buf_empty bufOps) as [] and (buf_append bufOps) as ++.

This code then essentially performs:

    foldr (flip (++)) [] xs

Which, if you look up the definition of foldr, is:

    ((([] ++ xN) ++ ... ) ++ x2) ++ x1

And a definition of ++ is of course:

    [] ++ ys = ys
    (x:xs) ++ ys = x : (xs ++ ys)

This means that for lists of this sort a ++ b runs in time O(length a), because it has to descend down the leftmost list to find the empty list -- only once it finds [] can it "work its way backwards" to append elements from a onto b.

If each of the x1, x2, ... xN has m elements, then we do 0 + m + 2m + ... + N m = m * N * (N + 1) / 2 operations. Each ++ will do about N operations and we'll do about N of them; it's O(N^2).

The new algorithm, `concat (reverse xs)`, works because `xs` is just a list which can be reversed by traversing down it in O(N) time, then those can be merged together in O(N * m) time.

zvrba3• 13 years ago

So the original line of code has apparently been present since the very first import by Sigbjørn Finne in https://github.com/nominolo/HTTP/blob/c4765e822eb92196fec955... (check line 443).

If a Haskell expert (e.g., he authored hdirect -- an IDL compiler and interface with Win32 COM -- sadly defunct now) makes this kind of mistake, how are mere mortals supposed to reason about algorithmic efficiency?

sjtgraham• 13 years ago

Humans occasionally make mistakes, including expert ones.

vanderZwan• 13 years ago

Exactly. Isn't it comforting to know that making mistakes is only human? And isn't this a wonderful practical example of why you want to be open source?

6d0debc071• 13 years ago

Advantage in focus - maybe he wrote it once and didn't look back at it. Low hanging fruit elsewhere for typical use case?

velis_vel2• 13 years ago

I don't think this was an 'oh my god we're trying to track down why cabal update is so slow and we just can't find it' issue like you make it out to be.

edit: http://www.reddit.com/r/haskell/comments/1sh67u/the_reason_w... This comment by the patch author indicates that actually tracking it down was a fairly straightforward profiling job.

zvrba3• 13 years ago

I didn't try to imply that it was difficult to find the performance problem. It's an observation on something else: Haskell is often mentioned for its equational reasoning. I find it interesting that correct equational reasoning in this case lead to a correct program, but one with absymal performance.

It'd be interesting to think how one could encode performance characteristics into equations.

microtherion2• 13 years ago

I suppose what Alan Perlis said about LISP programmers—that they know the value of everything and the cost of nothing—is true for Haskell programmers as well.

kovalkos• 13 years ago

This is a bad application of the quote.

Most of the Lisp hackers are usually knowledgeable about analysis of algorithm complexity.

Alan Perlis comment talks about inefficiency of Lisp programs due to a lot of (meta) abstraction and indirect mapping of abstract software to hardware.

hesselink• 13 years ago

I think back then, the 'n' just wasn't very large, so O(n) versus O(n^2) didn't make much of a difference.

nilkn• 13 years ago

Mistakes of this sort have been made in many languages. I don't see what this has to do with Haskell specifically.

eru6• 13 years ago

[...] how are mere mortals supposed to reason about algorithmic efficiency?

You profile, and then you look at the suspicious parts.

thomasahle• 13 years ago

Doesn't hlint catch stuff like this?

gwern• 13 years ago

How would hlint catch this? The offending line is

    foldr (flip (buf_append bufOps)) (buf_empty bufOps) strs

hlint doesn't know what 'buf_append bufOps' is, or 'buf_empty bufOps' or 'strs'.

If you saw 'foldr (flip (u v)) (x y) z', would you immediately go 'ah, n^2! better throw a warning'? If not, why do you expect hlint to warn?

taybin• 13 years ago

Yep, this can also bite you in Erlang.

Turn O(n^2) reverse into O(n)

Related Stories