Opened 5 years ago

Closed 3 years ago

#9696 closed bug (fixed)

readRawBufferPtr and writeRawBufferPtr allocate memory

Reported by: mergeconflict Owned by:
Priority: low Milestone: 8.2.1
Component: Compiler Version: 7.8.3
Keywords: Cc:
Operating System: Unknown/Multiple Architecture: x86_64 (amd64)
Type of failure: Runtime performance bug Test Case:
Blocked By: Blocking:
Related Tickets: Differential Rev(s): Phab:D2813
Wiki Page:

Description (last modified by mergeconflict)

I initially filed this as a question on StackOverflow, assuming that the behavior I'm seeing was intentional, or that I was misinterpreting the profiler results... but Kazu Yamamoto suggested I should file this as an issue here. It's my first GHC ticket, so here goes:

main :: IO ()
main = do
  buf <- mallocArray 1
  echo buf
 
echo :: Ptr Word8 -> IO ()
echo buf = forever $ do
  len <- readRawBufferPtr "read" stdin buf 0 1
  writeRawBufferPtr "write" stdout buf 0 (fromIntegral len)

I expect the only heap allocation here should be my explicit mallocArray, but profiling with +RTS -P indicates this isn't the case: both the read and write operations do appear to allocate some short-lived heap objects each time through the loop. (Note: this isn't a space leak, the allocated objects appear to be GC'ed quickly, whatever they are). In contrast:

echo :: Ptr Word8 -> IO ()
echo buf = forever $ do
  threadWaitRead $ Fd 0
  len <- c_read 0 buf 1
  c_write 1 buf (fromIntegral len)
  yield

does not appear to allocate.

I did a bit of digging (copying bits of source code from base into my own project, to get cost center annotations) and it seems like the allocation might be happening in throwErrnoIfRetryMayBlock. See this gist for more detail, including core output from -ddump-simpl.

In all honesty, I don't know whether this is a bug. I was just surprised by it, when I first encountered it using hGetBuf / hPutBuf, so I'm trying to understand whether this is expected behavior and why.

Change History (11)

comment:1 Changed 5 years ago by mergeconflict

Description: modified (diff)

comment:2 Changed 5 years ago by rwbarton

I don't think this is a big deal, but throwErrnoIfRetryMayBlock isn't getting inlined. It probably should get inlined.

Also, there is a safe foreign import of rtsSupportsBoundThreads in GHC.IO.FD (and also some other module(s) in base), which seems quite unnecessary considering rtsSupportsBoundThreads simply returns a constant.

comment:3 in reply to:  2 ; Changed 4 years ago by thomie

Keywords: newcomer added

Also, there is a safe foreign import of rtsSupportsBoundThreads in GHC.IO.FD (and also some other module(s) in base), which seems quite unnecessary considering rtsSupportsBoundThreads simply returns a constant.

For a newcomer, to get acquainted with the patch submission process.

comment:4 Changed 4 years ago by rwbarton

Priority: normallow

comment:5 in reply to:  3 Changed 4 years ago by mjmrotek

Replying to thomie:

Also, there is a safe foreign import of rtsSupportsBoundThreads in GHC.IO.FD (and also some other module(s) in base), which seems quite unnecessary considering rtsSupportsBoundThreads simply returns a constant.

For a newcomer, to get acquainted with the patch submission process.

I did this part in https://phabricator.haskell.org/D1964 , grepping through HEAD didn't show any more safe imports of rtsSupportsBoundThreads. I didn't touch anything related to readRawBufferPtr, writeRawBufferPtr, or throwErrnoIfRetryMayBlock, though.

comment:6 Changed 4 years ago by simonpj

Just to say, on the original problem, eliminating allocation from inner loops is a Good Goal. Sometimes it's hard (after all, GHC works largely with immutable values) but sometimes it's really quite accidental and easily eliminated.

I don't have time to dig in here; but inlining this throwErrNo thing might be helpful. On the other hand it may duplicate a lot of code; maybe it's worth asking why it is allocating and seeing what might avoid that need. E.g. if it has a higher order argument, perhaps specialising it for the various distinct call sites might help.

comment:7 Changed 4 years ago by Ben Gamari <ben@…>

In 1d6177b1/ghc:

Using unsafe foreign import for rtsSupportsBoundThreads (part of #9696)

A safe import is unnecessary considering rtsSupportsBoundThreads simply
returns a constant.

This commit doesn't fix the main issue of ticket #9696 that
"readRawBufferPtr and writeRawBufferPtr allocate memory".

Reviewers: bgamari, austin, hvr

Reviewed By: hvr

Subscribers: thomie

Differential Revision: https://phabricator.haskell.org/D1964

GHC Trac Issues: #9696

comment:8 Changed 3 years ago by thomie

Keywords: newcomer removed

comment:9 Changed 3 years ago by bgamari

Differential Rev(s): Phab:D2813
Milestone: 8.2.1
Status: newpatch

comment:10 Changed 3 years ago by Ben Gamari <ben@…>

In cc2e3ec0/ghc:

base: Make raw buffer IO operations more strict

Ticket #9696 reported that `readRawBufferPtr` and `writeRawBufferPtr`
allocated unnecessarily. The binding is question was,
```
let {
  buf_s4VD [Dmd=<L,U(U)>] :: GHC.Ptr.Ptr GHC.Word.Word8
  [LclId, Unf=OtherCon []] =
      NO_CCS GHC.Ptr.Ptr! [ds1_s4Vy];
} in
  case
      GHC.IO.FD.$wreadRawBufferPtr
          Main.main5
          0#
          0#
          buf_s4VD
          Main.main4
          Main.main3
          GHC.Prim.void#
  of ...
```
The problem was that GHC apparently couldn't tell that
`readRawBufferPtr` would always demand the buffer. Here we simple add
bang patterns on all of the small arguments of these functions to ensure
that worker/wrappers can eliminate these allocations.

Test Plan: Look at STG produced by testcase in #9696, verify no
allocations

Reviewers: austin, hvr, simonmar

Reviewed By: simonmar

Subscribers: RyanGlScott, simonmar, thomie

Differential Revision: https://phabricator.haskell.org/D2813

GHC Trac Issues: #9696

comment:11 Changed 3 years ago by bgamari

Resolution: fixed
Status: patchclosed
Note: See TracTickets for help on using tickets.