[darcs-users] darcs patch: Outsource the (optional) mmap support to bytestring-mmap.

Eric Kow kowey at darcs.net
Wed Jan 28 17:54:19 UTC 2009


Don,

Is there any chance you would be willing to review this patch for darcs?
Basically, what are the risks of this breaking darcs?

I'm hoping this will be a very small job for you since wrote both
bytestring-mmap (presumably starting from FastPackedString), and also
have looked at whittled FastPackedString down to ByteStringUtils during
sprint #1.

Thanks!

P.S., If not, I'll just ask Dmitry, but I thought maybe here you might
be particularly interested.

On Wed, Jan 28, 2009 at 18:43:38 +0100, Petr Rockai wrote:
> since there's this nice bytestring-mmap package by Don S., I propose to scrap
> our custom mmap code. It's ugly and also seems to break win32 on ghc6.10
> (cf. our windows buildslave).
> 
> The attached patch does that. It actually shouldn't break autoconf, just
> prevent autoconf-based builds from using mmap. Please also note that by
> default, the repository compression prevents useful mmaping anyway, so it's
> probably not such a big loss either.
> 
> Yours,
>    Petr.
> 
> Wed Jan 28 18:38:25 CET 2009  Petr Rockai <me at mornfall.net>
>   * Outsource the (optional) mmap support to bytestring-mmap.
> 

Content-Description: A darcs patch for your repository!
> 
> New patches:
> 
> [Outsource the (optional) mmap support to bytestring-mmap.
> Petr Rockai <me at mornfall.net>**20090128173825
>  Ignore-this: b497434fc44b93c41252f83464f08db2
> ] hunk ./Setup.lhs 167
>  
>    let features = [ ("HAVE_HTTP", "x-have-http" `elem` customFields)
>                   , ("USE_COLOR", "x-use-color" `elem` customFields)
> -                 , ("USE_MMAP", not isWindows)
>                   , ("HAVE_MAPI", isWindows)
>                   , ("HAVE_SENDMAIL", not $ null sendmail)
>                   , ("BIGENDIAN", bigendian) ]
> hunk ./darcs.cabal 116
>  flag color
>    description: Use ansi color escapes.
>  
> +flag mmap
> +  description: Compile with mmap support.
> +
>  flag base3
>  
>  Executable          witnesses
> hunk ./darcs.cabal 349
>          cpp-options:      -DHAVE_HTTP
>          x-have-http:
>  
> +  if flag(mmap)
> +    build-depends:    bytestring-mmap >= 0.2
> +    cpp-options:      -DHAVE_MMAP
> +
>    if flag(external-bytestring)
>      build-depends:    bytestring >= 0.9.0 && < 0.10
>      cpp-options:      -DHAVE_BYTESTRING
> hunk ./darcs.cabal 488
>          cpp-options:      -DHAVE_HTTP
>          x-have-http:
>  
> +  if flag(mmap)
> +    build-depends:    bytestring-mmap >= 0.2
> +    cpp-options:      -DHAVE_MMAP
> +
>    if flag(external-bytestring)
>      build-depends:    bytestring >= 0.9.0 && < 0.10
>      cpp-options:      -DHAVE_BYTESTRING
> hunk ./src/Autoconf.hs 10
>  {-# OPTIONS -cpp #-}
>  
>  module Autoconf ( have_libcurl, have_libwww, have_HTTP,
> -                  use_color, use_mmap, darcs_version, sendmail_path, have_sendmail,
> +                  use_color, darcs_version, sendmail_path, have_sendmail,
>                    have_mapi, diff_program,
>                    path_separator, big_endian,
>                  ) where
> hunk ./src/Autoconf.hs 49
>  use_color = False
>  #endif
>  
> -{-# INLINE use_mmap #-}
> -use_mmap :: Bool
> -#ifdef USE_MMAP
> -use_mmap = True
> -#else
> -use_mmap = False
> -#endif
> -
>  {-# INLINE sendmail_path #-}
>  sendmail_path :: String
>  sendmail_path = SENDMAIL
> hunk ./src/ByteStringUtils.hs 49
>          intercalate
>      ) where
>  
> -import Autoconf                 ( use_mmap )
> -
>  import qualified Data.ByteString            as B
>  import qualified Data.ByteString.Char8      as BC
>  #if __GLASGOW_HASKELL__ > 606
> hunk ./src/ByteStringUtils.hs 70
>  #endif
>  import Foreign.Marshal.Alloc    ( free )
>  import Foreign.Marshal.Array    ( mallocArray, peekArray, advancePtr )
> -import Foreign.C.Types          ( CInt, CSize )
> +import Foreign.C.Types          ( CInt )
>  
>  import Data.Bits                ( rotateL )
>  import Data.Char                ( chr, ord, isSpace )
> hunk ./src/ByteStringUtils.hs 81
>  import Foreign.Ptr              ( nullPtr, plusPtr, Ptr )
>  import Foreign.ForeignPtr       ( ForeignPtr, withForeignPtr )
>  
> -#if defined(__GLASGOW_HASKELL__)
> -import qualified Foreign.Concurrent as FC ( newForeignPtr )
> -import System.Posix             ( handleToFd )
> -#endif
> -
>  #ifdef DEBUG_PS
>  import Foreign.ForeignPtr       ( addForeignPtrFinalizer )
>  import Foreign.Ptr              ( FunPtr )
> hunk ./src/ByteStringUtils.hs 93
>  import Foreign.C.String ( CString, withCString )
>  #endif
>  
> +#ifdef HAVE_MMAP
> +import System.IO.Posix.MMap( unsafeMMapFile )
> +#endif
> +
>  -- -----------------------------------------------------------------------------
>  -- obsolete debugging code
>  
> hunk ./src/ByteStringUtils.hs 466
>  -- the file is assumed to be ISO-8859-1.
>  
>  mmapFilePS :: FilePath -> IO B.ByteString
> -mmapFilePS f = if use_mmap
> -               then do (fp,l) <- mmap f
> -                       return $ fromForeignPtr fp 0 l
> -               else B.readFile f
> -
> -#if defined(__GLASGOW_HASKELL__)
> -foreign import ccall unsafe "static fpstring.h my_mmap" my_mmap
> -    :: CSize -> CInt -> IO (Ptr Word8)
> -foreign import ccall unsafe "static sys/mman.h munmap" c_munmap
> -    :: Ptr Word8 -> CSize -> IO CInt
> -foreign import ccall unsafe "static unistd.h close" c_close
> -    :: CInt -> IO CInt
> -#endif
> -
> -mmap :: FilePath -> IO (ForeignPtr Word8, Int)
> -mmap f = do
> -    h <- openBinaryFile f ReadMode
> -    l <- fromIntegral `fmap` hFileSize h
> -    -- Don't bother mmaping small files because each mmapped file takes up
> -    -- at least one full VM block.
> -    if l < mmap_limit
> -       then do thefp <- BI.mallocByteString l
> -               debugForeignPtr thefp $ "mmap short file "++f
> -               withForeignPtr thefp $ \p-> hGetBuf h p l
> -               hClose h
> -               return (thefp, l)
> -       else do
> -#if defined(__GLASGOW_HASKELL__)
> -               fd <- fromIntegral `fmap` handleToFd h
> -               p <- my_mmap (fromIntegral l) fd
> -               fp <- if p == nullPtr
> -                     then
> +#ifdef HAVE_MMAP
> +mmapFilePS = unsafeMMapFile
>  #else
> hunk ./src/ByteStringUtils.hs 469
> -               fp <-
> +mmapFilePS = B.readFile
>  #endif
> hunk ./src/ByteStringUtils.hs 471
> -                          do thefp <- BI.mallocByteString l
> -                             debugForeignPtr thefp $ "mmap short file "++f
> -                             withForeignPtr thefp $ \p' -> hGetBuf h p' l
> -                             return thefp
> -#if defined(__GLASGOW_HASKELL__)
> -                     else do
> -                             fp <- FC.newForeignPtr p
> -                                   (do {c_munmap p $ fromIntegral l;
> -                                        return (); })
> -                             debugForeignPtr fp $ "mmap "++f
> -                             return fp
> -               c_close fd
> -#endif
> -               hClose h
> -               return (fp, l)
> -    where mmap_limit = 16*1024
> -
>  
>  -- -------------------------------------------------------------------------
>  -- fromPS2Hex
> hunk ./src/fpstring.c 62
>  
>  }
>  
> -// mmapping...
> -
> -#ifdef _WIN32
> -
> -/* I have no idea if this works or not, and it is very tied to the usage
> - * of mmap in FastPackedString. Most arguments are ignored...
> - */
> -
> -char *my_mmap(size_t length, int fd)
> -{
> -  exit(1); /* mmap is not implemented on Windows */
> -}
> -
> -int munmap(void *start, size_t length)
> -{
> -    UnmapViewOfFile(start);
> -}
> -
> -#else
> -
> -char *my_mmap(size_t len, int fd) {
> -  void *maybeok = mmap(NULL, len, PROT_READ, MAP_SHARED, fd, 0);
> -  if (maybeok == MAP_FAILED) return NULL;
> -  else return (char *)maybeok;
> -}
> -
> -#endif
> -
>  // ForeignPtr debugging stuff...
>  
>  static int num_alloced = 0;
> hunk ./src/fpstring.h 12
>  // int first_nonwhite(const char *s, int len);
>  int has_funky_char(const char *s, int len);
>  
> -char *my_mmap(size_t len, int fd);
> -
>  int utf8_to_ints(HsInt *pwc, const unsigned char *s, int n);
>  
>  void conv_to_hex(unsigned char *dest, unsigned char *from, int num_chars);

-- 
Eric Kow <http://www.nltg.brighton.ac.uk/home/Eric.Kow>
PGP Key ID: 08AC04F9
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: Digital signature
Url : http://lists.osuosl.org/pipermail/darcs-users/attachments/20090128/4a57cfd3/attachment.pgp 


More information about the darcs-users mailing list