Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reduce code bloat for literal strings by avoiding RULES and inlining #468

Merged
merged 2 commits into from
Aug 28, 2022

Conversation

Bodigrim
Copy link
Contributor

Closes #464.

What I don't really understand is why in spite of {-# NOINLINE unpackCStringAscii# #-} and {-# NOINLINE unpackCString# #-} GHC still applies worker-wrapper transformation:

-- RHS size: {terms: 1, types: 0, coercions: 0, joins: 0/0}
emojis1857 :: Addr#
emojis1857
  = "\\240\\159\\143\\180\\243\\160\\129\\167\\243\\160\\129\\162\\243\\160\\129\\183\\243\\160\\129\\172\\243\\160\\129\\179\\243\\160\\129\\191"#

-- RHS size: {terms: 8, types: 10, coercions: 0, joins: 0/0}
emojis1856 :: Text
emojis1856
  = case $wunpackCString# emojis1857 of
    { (# ww1_ahS0, ww2_ahS1, ww3_ahS2 #) ->
    Text ww1_ahS0 ww2_ahS1 ww3_ahS2
    }

-- RHS size: {terms: 1, types: 0, coercions: 0, joins: 0/0}
emojis1859 :: Addr#
emojis1859 = "wales"#

-- RHS size: {terms: 8, types: 10, coercions: 0, joins: 0/0}
emojis1858 :: Text
emojis1858
  = case $wunpackCStringAscii# emojis1859 of
    { (# ww1_ahS6, ww2_ahS7, ww3_ahS8 #) ->
    Text ww1_ahS6 ww2_ahS7 ww3_ahS8
    }

This does not happen with text-1.2.5.0:

-- RHS size: {terms: 1, types: 0, coercions: 0, joins: 0/0}
emojis1857 :: Addr#
emojis1857
  = "\\240\\159\\143\\180\\243\\160\\129\\167\\243\\160\\129\\162\\243\\160\\129\\183\\243\\160\\129\\172\\243\\160\\129\\179\\243\\160\\129\\191"#

-- RHS size: {terms: 2, types: 0, coercions: 0, joins: 0/0}
emojis1856 :: Text
emojis1856 = unpackCString# emojis1857

-- RHS size: {terms: 1, types: 0, coercions: 0, joins: 0/0}
emojis1859 :: Addr#
emojis1859 = "wales"#

-- RHS size: {terms: 2, types: 0, coercions: 0, joins: 0/0}
emojis1858 :: Text
emojis1858 = unpackCString# emojis1859

@AndreasPK any ideas?

@sjakobi does it help to reduce compile times?

CC @jgm @mpickering

@AndreasPK
Copy link

AndreasPK commented Aug 20, 2022

GHC generates workers for NOINLINE bindings. Which is also known to cause issues in some cases, see https://gitlab.haskell.org/ghc/ghc/-/issues/20364

It seems the old unpack version was just unstream (S.streamCString# addr#) which might have been opaque to GHC. GHC does a better job understanding the non-streaming version so does W/W to avoid allocating the Text constructor in certain situations.

I imagine this is actually beneficial for runtime performance when it helps. If that's often enough to be worth it I can not say.

@Bodigrim
Copy link
Contributor Author

Bodigrim commented Aug 20, 2022

@AndreasPK thanks, I did not know it. I guess it's not a big deal here: I imagine additional Core nodes should be reasonably fast to compile.

@AndreasPK
Copy link

@AndreasPK thanks, I did not know it. I guess it's not a big deal here: I imagine additional Core nodes should be reasonably fast to compile.

Yeah it should just be something like unpackC= case $wunpackC of (# x1, x2, x3 #) -> Text x1 x2 x3 which get's inlined.

@mpickering
Copy link
Contributor

I compiled the emojis package with this patch and it makes a massive difference.

before:

 102,867,674,224 bytes allocated in the heap
  33,195,407,952 bytes copied during GC
     587,046,336 bytes maximum residency (65 sample(s))
       2,699,840 bytes maximum slop
            1628 MiB total memory in use (0 MiB lost due to fragmentation)

                                     Tot time (elapsed)  Avg pause  Max pause
  Gen  0      1010 colls,     0 par   15.477s  15.510s     0.0154s    0.2033s
  Gen  1        65 colls,     0 par   24.926s  24.972s     0.3842s    0.7519s

  TASKS: 6 (1 bound, 5 peak workers (5 total), using -N1)

  SPARKS: 0 (0 converted, 0 overflowed, 0 dud, 0 GC'd, 0 fizzled)

  INIT    time    0.193s  (  0.193s elapsed)
  MUT     time   46.399s  ( 47.811s elapsed)
  GC      time   40.404s  ( 40.482s elapsed)
  EXIT    time    0.001s  (  0.008s elapsed)
  Total   time   86.996s  ( 88.493s elapsed)

after:

** Deleting temp dirs:
  20,667,160,256 bytes allocated in the heap
   4,245,384,896 bytes copied during GC
     145,651,272 bytes maximum residency (34 sample(s))
       1,995,288 bytes maximum slop
             372 MiB total memory in use (0 MiB lost due to fragmentation)

                                     Tot time (elapsed)  Avg pause  Max pause
  Gen  0       603 colls,     0 par    2.678s   2.701s     0.0045s    0.0339s
  Gen  1        34 colls,     0 par    2.390s   2.405s     0.0707s    0.1364s

  TASKS: 6 (1 bound, 5 peak workers (5 total), using -N1)

  SPARKS: 0 (0 converted, 0 overflowed, 0 dud, 0 GC'd, 0 fizzled)

  INIT    time    0.212s  (  0.222s elapsed)
  MUT     time   11.108s  ( 11.983s elapsed)
  GC      time    5.068s  (  5.107s elapsed)
  EXIT    time    0.001s  (  0.009s elapsed)
  Total   time   16.389s  ( 17.321s elapsed)

@Bodigrim Bodigrim requested a review from Lysxia August 26, 2022 21:28
@Bodigrim Bodigrim merged commit 5558730 into haskell:master Aug 28, 2022
@Bodigrim Bodigrim deleted the literal-bloat branch August 28, 2022 18:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Compile time increases related to changes between text-1.2.5.0 and text-2.0.1
4 participants