Re: [PATCH master] Fixup compatibility with GHC 7.4/base 4.5

['Iustin Pop' via ganeti-devel]

On Fri, Jun 10, 2016 at 12:07:45PM +0100, Brian Foley wrote:
> On Fri, Jun 10, 2016 at 12:33:23PM +0200, 'Iustin Pop' via ganeti-devel wrote:
> > On Fri, Jun 10, 2016 at 12:30:03PM +0200, Iustin Pop wrote:
> > > It looks like commit c429dd26 introduced the use of atomicModifyIORef', 
> > > which
> > > is only present in base 4.6 (GHC 7.6).  Let's temporarily fix that by 
> > > adding a
> > > small compat layer (which undoes the optimisations of using strict on 
> > > 7.4, but
> > > at least it works), until we decide to officially drop support for 7.4.
> > 
> > Forgot to add that I manually tested this on both ghc 7.4 (wheezy) and
> > 7.6.3.
> > 
> > iustin
> 
> Thanks.
> 
> I also noticed this problem later on when reading some the docs, but didn't
> get around to fixing it. Given the memory leak was pretty severe, could we 
> just
> put the implementation of atomicModifyIORef' from base-4.6.0.0 into the #ifdef
> rather than making it non-strict?
> 
> https://hackage.haskell.org/package/base-4.6.0.0/docs/src/Data-IORef.html#line-132

Good point. I chose the lazier way as I was not sure we care about
wheezy performance, but this is trivial enough indeed.

> Also, could you make this patch against stable-2.15 please (given that's where
> atomicModifyIORef' was introduced). I'll merge it forward to all the newer
> branches.

Sure thing! I looked at the commit and it was not included in any tag
(git describe --contains), so I thought it was done on master.

thanks,
iustin

Re: [PATCH master] Fixup compatibility with GHC 7.4/base 4.5

['Brian Foley' via ganeti-devel]

On Fri, Jun 10, 2016 at 12:33:23PM +0200, 'Iustin Pop' via ganeti-devel wrote:
> On Fri, Jun 10, 2016 at 12:30:03PM +0200, Iustin Pop wrote:
> > It looks like commit c429dd26 introduced the use of atomicModifyIORef', 
> > which
> > is only present in base 4.6 (GHC 7.6).  Let's temporarily fix that by 
> > adding a
> > small compat layer (which undoes the optimisations of using strict on 7.4, 
> > but
> > at least it works), until we decide to officially drop support for 7.4.
> 
> Forgot to add that I manually tested this on both ghc 7.4 (wheezy) and
> 7.6.3.
> 
> iustin

Thanks.

I also noticed this problem later on when reading some the docs, but didn't
get around to fixing it. Given the memory leak was pretty severe, could we just
put the implementation of atomicModifyIORef' from base-4.6.0.0 into the #ifdef
rather than making it non-strict?

https://hackage.haskell.org/package/base-4.6.0.0/docs/src/Data-IORef.html#line-132

Also, could you make this patch against stable-2.15 please (given that's where
atomicModifyIORef' was introduced). I'll merge it forward to all the newer
branches.

Cheers,
Brian.

Re: [PATCH master] Fixup compatibility with GHC 7.4/base 4.5

['Iustin Pop' via ganeti-devel]

On Fri, Jun 10, 2016 at 12:30:03PM +0200, Iustin Pop wrote:
> It looks like commit c429dd26 introduced the use of atomicModifyIORef', which
> is only present in base 4.6 (GHC 7.6).  Let's temporarily fix that by adding a
> small compat layer (which undoes the optimisations of using strict on 7.4, but
> at least it works), until we decide to officially drop support for 7.4.

Forgot to add that I manually tested this on both ghc 7.4 (wheezy) and
7.6.3.

iustin

[PATCH master] Fixup compatibility with GHC 7.4/base 4.5

['Iustin Pop' via ganeti-devel]

It looks like commit c429dd26 introduced the use of atomicModifyIORef', which
is only present in base 4.6 (GHC 7.6).  Let's temporarily fix that by adding a
small compat layer (which undoes the optimisations of using strict on 7.4, but
at least it works), until we decide to officially drop support for 7.4.

Signed-off-by: Iustin Pop 
---
 src/Ganeti/Compat.hs  | 10 ++
 src/Ganeti/JQScheduler.hs |  5 +++--
 2 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/src/Ganeti/Compat.hs b/src/Ganeti/Compat.hs
index e5276d1..3ee3293 100644
--- a/src/Ganeti/Compat.hs
+++ b/src/Ganeti/Compat.hs
@@ -41,10 +41,12 @@ module Ganeti.Compat
   ( rwhnf
   , Control.Parallel.Strategies.parMap
   , finiteBitSize
+  , atomicModifyIORef'
   ) where
 
 import qualified Control.Parallel.Strategies
 import qualified Data.Bits
+import qualified Data.IORef
 
 -- | Wrapper over the function exported from
 -- "Control.Parallel.Strategies".
@@ -67,3 +69,11 @@ finiteBitSize :: (Data.Bits.FiniteBits a) => a -> Int
 finiteBitSize = Data.Bits.finiteBitSize
 #endif
 {-# INLINE finiteBitSize #-}
+
+-- FIXME: remove this when dropping support for GHC 7.4.
+atomicModifyIORef' :: Data.IORef.IORef a -> (a -> (a, b)) -> IO b
+#if MIN_VERSION_base(4,6,0)
+atomicModifyIORef' = Data.IORef.atomicModifyIORef'
+#else
+atomicModifyIORef' = Data.IORef.atomicModifyIORef
+#endif
diff --git a/src/Ganeti/JQScheduler.hs b/src/Ganeti/JQScheduler.hs
index 2eae077..9d0f72d 100644
--- a/src/Ganeti/JQScheduler.hs
+++ b/src/Ganeti/JQScheduler.hs
@@ -64,7 +64,7 @@ import Control.Monad ( when
  , forM_)
 import Control.Monad.IO.Class
 import Data.Function (on)
-import Data.IORef
+import Data.IORef (IORef, atomicModifyIORef, newIORef, readIORef)
 import Data.List ( find
  , deleteFirstsBy
  , sortBy
@@ -79,6 +79,7 @@ import qualified Data.Set as S
 import System.INotify
 
 import Ganeti.BasicTypes
+import Ganeti.Compat
 import Ganeti.Constants as C
 import Ganeti.Errors
 import Ganeti.JQScheduler.Filtering (applyingFilter, jobFiltering)
@@ -449,7 +450,7 @@ scheduleSomeJobs qstate = do
 showQueue :: Queue -> String
 showQueue (Queue {qEnqueued=waiting, qRunning=running}) =
   let showids = show . map (fromJobId . qjId . jJob)
-  in "Waiting jobs: " ++ showids waiting 
+  in "Waiting jobs: " ++ showids waiting
++ "; running jobs: " ++ showids running
 
 -- | Check if a job died, and clean up if so. Return True, if
-- 
2.8.0.rc3.226.g39d4020