Skip to content
This repository was archived by the owner on Sep 20, 2023. It is now read-only.

Conversation

NicolasDP
Copy link
Member

instead of #205

improved the Block bench of about 200 μs

Test Mean
types/ByteArray/Monoid/mconcat/listBs20/[UArray_W8] 541.4 ns ( +- 8.050 ns )
types/ByteArray/Monoid/mconcat/listBs20/[Block_W8] 185.4 ns ( +- 1.653 ns )
types/ByteArray/Monoid/mconcat/listBs20/[ByteString] 162.9 ns ( +- 1.422 ns )
types/ByteArray/Monoid/mconcat/listBs20/[Vector_W8] 228.9 ns ( +- 1.219 ns )
types/ByteArray/Monoid/mconcat/listBs200/[UArray_W8] 6.728 μs ( +- 107.6 ns )
types/ByteArray/Monoid/mconcat/listBs200/[Block_W8] 3.887 μs ( +- 38.41 ns )
types/ByteArray/Monoid/mconcat/listBs200/[ByteString] 3.400 μs ( +- 110.7 ns )
types/ByteArray/Monoid/mconcat/listBs200/[Vector_W8] 3.817 μs ( +- 47.31 ns )
types/ByteArray/Monoid/mconcat/listBs2000/[UArray_W8] 937.6 μs ( +- 36.67 μs )
types/ByteArray/Monoid/mconcat/listBs2000/[Block_W8] 692.0 μs ( +- 24.46 μs )
types/ByteArray/Monoid/mconcat/listBs2000/[ByteString] 581.1 μs ( +- 19.47 μs )
types/ByteArray/Monoid/mconcat/listBs2000/[Vector_W8] 932.0 μs ( +- 19.45 μs )
types/ByteArray/Monoid/builder/[block Word8]* 733.3 μs ( +- 27.22 μs )

*this last bench is interesting to compare the little difference between this new implementation of concat and using builder added in #455

I just did this quickly and I think there is still room for improvement (I believe we can go faster or at least as fast as bytestring). Below is an quote from the core dump of the function where I believe we might be able to improve things, here the Offset is not unpacked in the code and we have a case to access it, this might be the source of the performance issue here:

                          case ds1_d1dZF
                               `cast` (Basement.Types.OffsetSize.N:Offset[0] <Word8>_P
                                       :: (Offset Word8 :: *) ~R# (Int :: *))
                          of
                          { I# ipv4_s1eaO ->
                          case ipv2_s1eaJ of { Block ba_a1cMZ ->
                          let {
                            ipv5_s1eaS [Dmd=<S,U>] :: Int#
                            [LclId]
                            ipv5_s1eaS = sizeofByteArray# ba_a1cMZ } in
                          case copyByteArray#
                                 @ (PrimState (ST RealWorld))
                                 ba_a1cMZ
                                 0#
                                 ipv1_s1e8T
                                 ipv4_s1eaO
                                 ipv5_s1eaS

I wonder what else I could do to help GHC understand it can optimise away down to I# like it does for computing the length of the Block to create:

Basement.Block.Base.$wpoly_size
  = \ (@ ty_s1fkN)
      (ww_s1fkS :: Int#)
      (w_s1fkP :: [Block ty_s1fkN]) ->
      case w_s1fkP of {
        [] -> ww_s1fkS;
        : x_a1cO9 xs_a1cOa ->
          case x_a1cO9 of { Block ba_a1cMZ ->
          Basement.Block.Base.$wpoly_size
            @ ty_s1fkN (+# (sizeofByteArray# ba_a1cMZ) ww_s1fkS) xs_a1cOa
          }
      }

@NicolasDP NicolasDP added X - help wanted Need help from someone C - performance performance related T - array affect type Arrays labels Jan 13, 2018
@NicolasDP NicolasDP requested a review from vincenthz January 13, 2018 09:04
@vincenthz vincenthz changed the title Block/UArray/Builder: concat upgrade Block/UArray/Builder: concat performance update Jan 13, 2018
@vincenthz
Copy link
Member

seems an improvement already, but it's likely we could move to keep track of values in bytes instead of unit of ty for the UArray. For the Offset unboxing, I think looking at some other point is fine

@vincenthz vincenthz merged commit 1b4ba7a into master Jan 13, 2018
Copy link
Member

@vincenthz vincenthz left a comment

Choose a reason for hiding this comment

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

👍

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
C - performance performance related T - array affect type Arrays X - help wanted Need help from someone
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants