Tail-recursive function consuming memory

Question

I have a clearly tail-recursive function for finding (choose n k) mod 10007 (with k nonnegative)

Why is this function consuming lots of memory for large inputs? (ie 100000000 choose 50000000) I can understand if it might be slow, but it shouldn't use more than constant memory, should it? (assuming GHC knows about tail-call optimization)

GHC version 7.8.3

modulus :: Int
modulus = 10007

choose :: Int -> Int -> Int
choose n1 k1
    | s1 > 0 = 0
    | otherwise = q1
  where
    (q1, s1) = doChoose n1 k1 (1, 0)
    doChoose :: Int -> Int -> (Int, Int) -> (Int, Int)
    doChoose _ 0 (qr, sr) = (qr, sr)
    doChoose n k (qr, sr) =
        doChoose (n `seq` (n-1)) (k-1) (qr `seq` (qn * qr `rem` modulus * inv qk `rem` modulus), sr `seq` (sn + sr - sk))
      where
        (qn, sn) = removePs n
        (qk, sk) = removePs k

removePs :: Int -> (Int, Int)
removePs n =
  case r of
    0 -> (q0, s0 + 1)
    _ -> (n, 0)
  where
    (q, r) = n `quotRem` modulus
    (q0, s0) = removePs q

inv :: Int -> Int
inv = doInv 0 1 modulus . (`mod` modulus)
  where
    doInv x _ 1 0
      | x < 0 = x + modulus
      | otherwise = x
    doInv _ _ _ 0 = error "Not relatively prime"
    doInv x y a b = doInv y (x - q * y) b r
      where
        (q, r) = a `quotRem` b

You appear to be building huge thunks for the values of `n`, `qr` and `sr`. Try using bang patterns or `seq`. — Thomas M. DuBuisson, Nov 27 '14 at 00:30
@ThomasM.DuBuisson I am using `seq` for qr and sr. They shouldn't be building up. Although it didn't occur to me to do n as well. I fixed it but it's still consuming lots of memory — dspyz, Nov 27 '14 at 01:34
Oh, I see, I'm putting the seq in the wrong place. I need to put it before the recursive call or the whole thing will just be left as a thunk — dspyz, Nov 27 '14 at 01:46
I think you might find that wondering about the performance characteristics of your code not compiled with `-O2` is counter-productive. The strictness annotations you have (correctly below) are a good idea, but you might find it's nicer to trust GHC to get strictness right and add annotations later after profiling. — jberryman, Nov 27 '14 at 04:21

dspyz · Accepted Answer · 2014-11-27T01:55:57.410

I was putting the seq in the wrong place.

It needs to be:

n `seq` qr `seq` sr `seq` doChoose (n-1) (k-1) (qn * qr `rem` modulus * inv qk `rem` modulus, sn + sr - sk)

Otherwise the call to seq isn't evaluated until reaching the base-case and a chain of thunks is still built up.

This isn't strictly tail-recursive, but rather it's "mutually" tail-recursive since seq ultimately returns its second argument without modifying it.

score -1 · Answer 2 · answered Nov 27 '14 at 14:30

-1

By the way, to simplify your expressions, you can write a helper function:

force x = x `seq` x

or use force (no pun intended) from the Deepseq package. Then

doChoose (force n - 1) (k - 1) (qn * force qr * etc.)

answered Nov 27 '14 at 14:30

Yuuri

1,858
1
16
26

1

This is wrong. See one of my previous haskell questions: http://stackoverflow.com/questions/23570589/would-you-ever-write-seq-x-x – dspyz Nov 28 '14 at 01:48
You're right, sorry. At least `deepseq` should work. – Yuuri Nov 28 '14 at 10:10

Tail-recursive function consuming memory

2 Answers2