[GHC] #13246: hPutBuf issues unnecessary empty write syscalls for large writes

#13246: hPutBuf issues unnecessary empty write syscalls for large writes -------------------------------------+------------------------------------- Reporter: nh2 | Owner: Type: bug | Status: new Priority: normal | Milestone: 8.2.1 Component: Runtime | Version: 8.0.2 System | Keywords: | Operating System: Unknown/Multiple Architecture: | Type of failure: Runtime Unknown/Multiple | performance bug Test Case: | Blocked By: Blocking: | Related Tickets: #13245 Differential Rev(s): | Wiki Page: -------------------------------------+------------------------------------- To get good performance, it is better to use few system calls that write lots of data in batch. I found a bug in hPutBuf that makes this concept not work: When using `hPutBuf` with a buffer size greater than 8095 (this number it self is a bug, #13245) bytes, two syscalls are issued instead of one: one empty `write("")` (a zero-bytes-write, which can't do anything useful), and after that the actual useful `write()` of the data. Example code: {{{ main = do withBinaryFile "testfile2" WriteMode $ \ hTo -> do let bufferSize = 8096 allocaBytes bufferSize $ \buffer -> do Main.hPutBuf hTo buffer bufferSize }}} In `strace -f -T` on the compiled binary, we see the syscalls: {{{ write(3, "", 0) = 0 <0.000004> write(3, "\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"..., 8096) = 8096 <0.000017> }}} As you can see in the timings, this also has a fairly large performance overhead (20% in this case). When using `bufferSize = 8095`, the `write("")` disappears. The problem is this code for `bufWrite` (called by `hPutBuf`): {{{ bufWrite :: Handle__-> Ptr Word8 -> Int -> Bool -> IO Int bufWrite h_@Handle__{..} ptr count can_block = seq count $ do -- strictness hack old_buf@Buffer{ bufRaw=old_raw, bufR=w, bufSize=size } <- readIORef haByteBuffer -- enough room in handle buffer? hPutStrLn System.IO.stderr (show (size, w, count)) if (size - w > count) -- There's enough room in the buffer: -- just copy the data in and update bufR. then do debugIO ("hPutBuf: copying to buffer, w=" ++ show w) copyToRawBuffer old_raw w ptr count writeIORef haByteBuffer old_buf{ bufR = w + count } return count -- else, we have to flush else do debugIO "hPutBuf: flushing first" old_buf' <- Buffered.flushWriteBuffer haDevice old_buf -- TODO: we should do a non-blocking flush here writeIORef haByteBuffer old_buf' -- if we can fit in the buffer, then just loop if count < size then bufWrite h_ ptr count can_block else if can_block then do writeChunk h_ (castPtr ptr) count return count else writeChunkNonBlocking h_ (castPtr ptr) count }}} The check `if (size - w > count)` should be `if (size - w >= count)` instead, because we can do the write all fine if it fits exactly. In the adversarial case, `size - w == count`, we go into the `hPutBuf: flushing first` branch, thus emitting the `write("")`. See https://github.com/ghc/ghc/blame/876b00ba25a615423f48b0cf9d443a9fd5dbd6f4/li... for the full code. Simon Marlow has confirmed this on IRC, I'll submit a patch for it that switches to `>=`. It would be nice if the fix could be released in both GHC 8.2 and and 8.0.3. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/13246 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#13246: hPutBuf issues unnecessary empty write syscalls for large writes -------------------------------------+------------------------------------- Reporter: nh2 | Owner: Type: bug | Status: new Priority: normal | Milestone: 8.2.1 Component: Runtime System | Version: 8.0.2 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Type of failure: Runtime | Unknown/Multiple performance bug | Test Case: Blocked By: | Blocking: Related Tickets: #13245 | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Comment (by nh2): I've extracted the relevant code into a simple-to-run-and-modify single file, so that we can play around with it without recompiling `base`: * [https://gist.github.com/nh2/4cd40c2a4b15bf056bcae87907773786#file-ghc- bug-13246-repro-hs Without fix] * [https://gist.github.com/nh2/4cd40c2a4b15bf056bcae87907773786#file-ghc- bug-13246-fix-hs With fix] -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/13246#comment:1 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#13246: hPutBuf issues unnecessary empty write syscalls for large writes -------------------------------------+------------------------------------- Reporter: nh2 | Owner: Type: bug | Status: new Priority: normal | Milestone: 8.2.1 Component: Runtime System | Version: 8.0.2 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Type of failure: Runtime | Unknown/Multiple performance bug | Test Case: Blocked By: | Blocking: Related Tickets: #13245 | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Comment (by nh2): When writing the fix, I noticed that my description of what exactly is the bug is wrong. The `if (size - w > count)` isn't the problem, that's just another infelicity (e.g. it'll make that if you have an 8 K buffer and write 2 K chunks to it, we'll always flush 6 K buffers), which I'll fix too. The real bug is simply that we `flushWriteBuffer` even if `w == 0` (handle buffer is empty). -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/13246#comment:2 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#13246: hPutBuf issues unnecessary empty write syscalls for large writes -------------------------------------+------------------------------------- Reporter: nh2 | Owner: Type: bug | Status: new Priority: normal | Milestone: 8.2.1 Component: Runtime System | Version: 8.0.2 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Type of failure: Runtime | Unknown/Multiple performance bug | Test Case: Blocked By: | Blocking: Related Tickets: #13245 | Differential Rev(s): Wiki Page: | https://phabricator.haskell.org/D3117 -------------------------------------+------------------------------------- Changes (by nh2): * differential: => https://phabricator.haskell.org/D3117 -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/13246#comment:3 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#13246: hPutBuf issues unnecessary empty write syscalls for large writes -------------------------------------+------------------------------------- Reporter: nh2 | Owner: Type: bug | Status: patch Priority: normal | Milestone: 8.2.1 Component: Runtime System | Version: 8.0.2 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Type of failure: Runtime | Unknown/Multiple performance bug | Test Case: Blocked By: | Blocking: Related Tickets: #13245 | Differential Rev(s): Wiki Page: | https://phabricator.haskell.org/D3117 -------------------------------------+------------------------------------- Changes (by nh2): * status: new => patch -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/13246#comment:4 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#13246: hPutBuf issues unnecessary empty write syscalls for large writes -------------------------------------+------------------------------------- Reporter: nh2 | Owner: Type: bug | Status: patch Priority: normal | Milestone: 8.2.1 Component: Runtime System | Version: 8.0.2 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Type of failure: Runtime | Unknown/Multiple performance bug | Test Case: Blocked By: | Blocking: Related Tickets: #13245 | Differential Rev(s): Phab:D3117 Wiki Page: | -------------------------------------+------------------------------------- Changes (by mpickering): * differential: https://phabricator.haskell.org/D3117 => Phab:D3117 -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/13246#comment:5 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#13246: hPutBuf issues unnecessary empty write syscalls for large writes
-------------------------------------+-------------------------------------
Reporter: nh2 | Owner:
Type: bug | Status: patch
Priority: normal | Milestone: 8.2.1
Component: Runtime System | Version: 8.0.2
Resolution: | Keywords:
Operating System: Unknown/Multiple | Architecture:
Type of failure: Runtime | Unknown/Multiple
performance bug | Test Case:
Blocked By: | Blocking:
Related Tickets: #13245 | Differential Rev(s): Phab:D3117
Wiki Page: |
-------------------------------------+-------------------------------------
Comment (by Ben Gamari

#13246: hPutBuf issues unnecessary empty write syscalls for large writes -------------------------------------+------------------------------------- Reporter: nh2 | Owner: Type: bug | Status: merge Priority: normal | Milestone: 8.0.3 Component: Runtime System | Version: 8.0.2 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Type of failure: Runtime | Unknown/Multiple performance bug | Test Case: Blocked By: | Blocking: Related Tickets: #13245 | Differential Rev(s): Phab:D3117 Wiki Page: | -------------------------------------+------------------------------------- Changes (by bgamari): * status: patch => merge * milestone: 8.2.1 => 8.0.3 Comment: At the moment we don't have a compelling reason to release a 8.0.3 but I'll milestone it there regardless. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/13246#comment:7 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#13246: hPutBuf issues unnecessary empty write syscalls for large writes -------------------------------------+------------------------------------- Reporter: nh2 | Owner: (none) Type: bug | Status: closed Priority: normal | Milestone: 8.2.1 Component: Runtime System | Version: 8.0.2 Resolution: fixed | Keywords: Operating System: Unknown/Multiple | Architecture: Type of failure: Runtime | Unknown/Multiple performance bug | Test Case: Blocked By: | Blocking: Related Tickets: #13245 | Differential Rev(s): Phab:D3117 Wiki Page: | -------------------------------------+------------------------------------- Changes (by bgamari): * status: merge => closed * resolution: => fixed -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/13246#comment:9 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
participants (1)
-
GHC