More elegant version of match2. SLOWDOWN by a factor of 100! :-(

1 parent 9cf0ace commit b6f58699258ef68ddee21a1346bd184465aaaba2 committed Sep 3, 2012
Showing with 8 additions and 9 deletions.
 @@ -120,10 +120,10 @@ narrow seed state = if Set.null seed then Just state else \$ filter ((matchl (r, c+1) state).right) \$ filter ((matchl (r+1, c) state).bottom) \$ filter ((matchl (r, c-1) state).left) - \$ filter (\x -> match2 (r-1, c-1) state bottom (left x) right (top x)) - \$ filter (\x -> match2 (r-1, c+1) state bottom (right x) left (top x )) - \$ filter (\x -> match2 (r+1, c-1) state top (left x) right (bottom x)) - \$ filter (\x -> match2 (r+1, c+1) state top (right x) left (bottom x)) + \$ filter (match2 (r-1, c-1) state [(bottom, left), (right, top)]) + \$ filter (match2 (r-1, c+1) state [(bottom, right), (left, top)]) + \$ filter (match2 (r+1, c-1) state [(top, left), (right, bottom)]) + \$ filter (match2 (r+1, c+1) state [(top, right), (left, bottom)]) ss if null ss' then Nothing @@ -145,11 +145,10 @@ matchl i state x = if inRange (bounds state) i where check (Line ls _) = x `elem` ls check _ = undefined -- can't happen -match2 :: (Int, Int) -> State -> (FourLines -> Bool) -> Bool -> (FourLines -> Bool) -> Bool -> Bool -match2 i state f1 x1 f2 x2 = (not (inRange (bounds state) i)) - || check (state!i) - where check (Space xs) = any (\x -> f1 x == x1 && f2 x == x2) xs - check _ = undefined -- can't happen +match2 :: (Int, Int) -> State -> [(FourLines->Bool, FourLines->Bool)] -> FourLines -> Bool +match2 i state fps thiscell = (not (inRange (bounds state) i)) || all pairmatch fps + where pairmatch (otherf, thisf) = any ((==thisf thiscell) . otherf) othercell + Space othercell = state!i narrowAll :: State -> Maybe State narrowAll state = narrow (Set.fromList (indices state)) state

#### 6 comments on commit `b6f5869`

Owner
commented on `b6f5869` Sep 3, 2012
 This change made the program dramatically slower and I don't understand why. For the (default) 10x10 test case this version needs 55 seconds while the version in the parent commit needs .33 seconds on my machine. I thought I had made only a cosmetic change expressing things more elegantly. Any ideas why there is such a huge performance impact would be greatly appreciated.
commented on `b6f5869` Sep 3, 2012
 Did you compile with -O2? Just a wild guess but optimization may have quite a drastic effect with GHC.
commented on `b6f5869` Sep 3, 2012
 From a first glance I notice that in the original code, there is `any (... && ...) xs` while the new code has `all (... any ... xs) fps`, so you are reversing the conjunction and disjunction. Not sure if it is sound, but maybe that already caused the difference. Is `othercell` large?
Owner
commented on `b6f5869` Sep 4, 2012
 Thanks a lot, @nomeata ! You are absolutely right and that was the problem. I have fixed match2 and it is now fast again.
Owner
commented on `b6f5869` Sep 4, 2012
 And thanks to @skogsbaer too. Yes, I am compiling with -O2.
commented on `b6f5869` Sep 4, 2012
 Right: "any" terminates after the first match, while "all" has to scan the entire list.