Skip to content

Improve Distribution.Lex.tokenizeQuotedWords - #12187

Open
Bodigrim wants to merge 1 commit into
masterfrom
faster-tokenizeQuotedWords
Open

Improve Distribution.Lex.tokenizeQuotedWords#12187
Bodigrim wants to merge 1 commit into
masterfrom
faster-tokenizeQuotedWords

Conversation

@Bodigrim

Copy link
Copy Markdown
Collaborator

The new version is more concise, 25% faster and allocates 50% less.

(The reason of my interest to this function is that I'm investigating whether any usages of DList throughout Cabal code are up to the purpose and not slowing things down)

Tested and benchmarked using the following program:

{- cabal:
build-depends: base, dlist, tasty, tasty-quickcheck, tasty-bench
-}

{-# LANGUAGE LambdaCase #-}

import Data.Char (isSpace)
import Data.DList (DList(..), toList, singleton)
import Data.List (replicate)
import Data.List.NonEmpty (NonEmpty(..))
import Test.Tasty.Bench (defaultMain, bench, nf)
import Test.Tasty.QuickCheck

tokenizeQuotedWords :: String -> [String]
tokenizeQuotedWords = filter (not . null) . go False mempty
  where
    go
      :: Bool
      -- \^ in quoted region
      -> DList Char
      -- \^ accumulator
      -> String
      -- \^ string to be parsed
      -> [String]
    -- \^ parse result
    go _ accum []
      | [] <- accum' = []
      | otherwise = [accum']
      where
        accum' = toList accum
    go False accum (c : cs)
      | isSpace c = toList accum : go False mempty cs
      | c == '"' = go True accum cs
    go True accum (c : cs)
      | c == '"' = go False accum cs
    go quoted accum (c : cs) =
      go quoted (accum <> singleton c) cs

tokenizeQuotedWords2 :: String -> [String]
tokenizeQuotedWords2 xs = repack $ foldr alg (const ([] :| [])) xs False
  where
    repack :: NonEmpty String -> [String]
    repack (zs :| acc) = if null zs then acc else zs : acc

    alg :: Char -> (Bool -> NonEmpty String) -> Bool -> NonEmpty String
    alg '"' rest mode = rest (not mode)
    alg c rest False
      | isSpace c = [] :| repack (rest False)
    alg c rest mode = case rest mode of
        w :| ws -> (c : w) :| ws

data Foo = Space | Quote | CharA | CharB
  deriving (Eq, Ord, Show, Bounded, Enum)

instance Arbitrary Foo where
  arbitrary = arbitraryBoundedEnum

fooToChar :: Foo -> Char
fooToChar = \case
  Space -> ' '
  Quote -> '"'
  CharA -> 'a'
  CharB -> 'b'

main :: IO ()
main = defaultMain
  [ testProperty "same results" $
      \xs -> let ys = map fooToChar xs in tokenizeQuotedWords ys === tokenizeQuotedWords2 ys
  , bench "old" $ nf tokenizeQuotedWords benchString
  , bench "new" $ nf tokenizeQuotedWords2 benchString
  ]

benchString :: String
benchString = concat $ replicate 10 "foo \"bar baz\" quux"

Template B: This PR does not modify behaviour or interface

Include the following checklist in your PR:

  • Patches conform to the coding conventions.
  • Is this a PR that fixes CI? If so, it will need to be backported to older cabal release branches (ask maintainers for directions).

@Bodigrim
Bodigrim force-pushed the faster-tokenizeQuotedWords branch from 572d642 to 8b57558 Compare July 30, 2026 21:37
@Bodigrim
Bodigrim force-pushed the faster-tokenizeQuotedWords branch from 8b57558 to 2d0257a Compare August 4, 2026 22:16
Comment thread Cabal/src/Distribution/Lex.hs Outdated
repack :: NonEmpty String -> [String]
repack (zs :| acc) = if null zs then acc else zs : acc

alg :: Char -> (Bool -> NonEmpty String) -> Bool -> NonEmpty String

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the previous version had nice haddocks on parameters of go, which helped a lot with understanding this code. Could you please follow that lead?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, done.

The new version is more concise, 25% faster and allocates 50% less.

(The reason of my interest to this function is that I'm investigating
whether any usages of `DList` throughout Cabal code are up to the purpose
and not slowing things down)

Tested and benchmarked using the following program:

```haskell
{- cabal:
build-depends: base, dlist, tasty, tasty-quickcheck, tasty-bench
-}

{-# LANGUAGE LambdaCase #-}

import Data.Char (isSpace)
import Data.DList (DList(..), toList, singleton)
import Data.List (replicate)
import Data.List.NonEmpty (NonEmpty(..))
import Test.Tasty.Bench (defaultMain, bench, nf)
import Test.Tasty.QuickCheck

tokenizeQuotedWords :: String -> [String]
tokenizeQuotedWords = filter (not . null) . go False mempty
  where
    go
      :: Bool
      -- \^ in quoted region
      -> DList Char
      -- \^ accumulator
      -> String
      -- \^ string to be parsed
      -> [String]
    -- \^ parse result
    go _ accum []
      | [] <- accum' = []
      | otherwise = [accum']
      where
        accum' = toList accum
    go False accum (c : cs)
      | isSpace c = toList accum : go False mempty cs
      | c == '"' = go True accum cs
    go True accum (c : cs)
      | c == '"' = go False accum cs
    go quoted accum (c : cs) =
      go quoted (accum <> singleton c) cs

tokenizeQuotedWords2 :: String -> [String]
tokenizeQuotedWords2 xs = repack $ foldr alg (const ([] :| [])) xs False
  where
    repack :: NonEmpty String -> [String]
    repack (zs :| acc) = if null zs then acc else zs : acc

    alg :: Char -> (Bool -> NonEmpty String) -> Bool -> NonEmpty String
    alg '"' rest mode = rest (not mode)
    alg c rest False
      | isSpace c = [] :| repack (rest False)
    alg c rest mode = case rest mode of
        w :| ws -> (c : w) :| ws

data Foo = Space | Quote | CharA | CharB
  deriving (Eq, Ord, Show, Bounded, Enum)

instance Arbitrary Foo where
  arbitrary = arbitraryBoundedEnum

fooToChar :: Foo -> Char
fooToChar = \case
  Space -> ' '
  Quote -> '"'
  CharA -> 'a'
  CharB -> 'b'

main :: IO ()
main = defaultMain
  [ testProperty "same results" $
      \xs -> let ys = map fooToChar xs in tokenizeQuotedWords ys === tokenizeQuotedWords2 ys
  , bench "old" $ nf tokenizeQuotedWords benchString
  , bench "new" $ nf tokenizeQuotedWords2 benchString
  ]

benchString :: String
benchString = concat $ replicate 10 "foo \"bar baz\" quux"
```
@Bodigrim
Bodigrim force-pushed the faster-tokenizeQuotedWords branch from 2d0257a to e81baa7 Compare August 5, 2026 19:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants