-
Notifications
You must be signed in to change notification settings - Fork 280
warp/recv: optimistic non-blocking recv before parking the thread #1099
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,14 +1,22 @@ | ||
| {-# LANGUAGE CPP #-} | ||
| {-# LANGUAGE OverloadedStrings #-} | ||
|
|
||
| module Network.Socket.BufferPool.Recv ( | ||
| receive, | ||
| receiveNoWait, | ||
| makeRecvN, | ||
| ) where | ||
|
|
||
| import qualified Data.ByteString as BS | ||
| import Data.ByteString.Internal (ByteString (..), unsafeCreate) | ||
| import Data.IORef | ||
| import Network.Socket (Socket, recvBuf) | ||
| #ifndef mingw32_HOST_OS | ||
| import Foreign.C.Types (CInt (..), CSize (..)) | ||
| import Foreign.Ptr (Ptr, castPtr) | ||
| import Network.Socket (withFdSocket) | ||
| import System.Posix.Types (CSsize (..)) | ||
| #endif | ||
|
|
||
| import Network.Socket.BufferPool.Buffer | ||
| import Network.Socket.BufferPool.Types | ||
|
|
@@ -20,6 +28,27 @@ import Network.Socket.BufferPool.Types | |
| receive :: Socket -> BufferPool -> Recv | ||
| receive sock pool = withBufferPool pool $ \ptr size -> recvBuf sock ptr size | ||
|
|
||
| -- | Like 'receive' but never blocks and never involves the IO manager: | ||
| -- 'Nothing' means no data was available (or an error occurred, which a | ||
| -- subsequent blocking 'receive' will report properly). @Just \"\"@ is EOF. | ||
| -- On Windows this always returns 'Nothing'. | ||
| receiveNoWait :: Socket -> BufferPool -> IO (Maybe ByteString) | ||
| #ifndef mingw32_HOST_OS | ||
| receiveNoWait sock pool = tryWithBufferPool pool $ \ptr size -> | ||
| withFdSocket sock $ \fd -> do | ||
| -- The socket is non-blocking, so an unsafe foreign call is fine: | ||
| -- recv(2) returns immediately either way. Both EAGAIN and real | ||
| -- errors map to a negative result, deferring to the blocking path | ||
| -- so errors surface there with their errno intact. | ||
| n <- c_recv fd (castPtr ptr) (fromIntegral size) 0 | ||
| return $ fromIntegral n | ||
|
|
||
| foreign import ccall unsafe "recv" | ||
| c_recv :: CInt -> Ptr CSsize -> CSize -> CInt -> IO CSsize | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We basically want to use
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If necessary, I can export it from
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think that in itself is a good idea. It's nice for the user to be able to decide to block or not block on a |
||
| #else | ||
| receiveNoWait _ _ = return Nothing | ||
| #endif | ||
|
|
||
| ---------------------------------------------------------------- | ||
|
|
||
| -- | This function returns a receiving function | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -144,22 +144,33 @@ socketConnection set s = do | |
| -- actively using this 'Socket'. | ||
| makeGracefulRecv :: Socket -> BufferPool -> ServerState -> TVar Int -> Recv | ||
| makeGracefulRecv sock pool ss appsInProgress = do | ||
| sockWait <- | ||
| -- Fast path: under load the next request has usually already arrived, | ||
| -- so read it without registering with the IO manager or entering STM. | ||
| -- One epoll_ctl + one futex sleep/wake pair saved per request. | ||
| -- A request already in the kernel buffer is served even if shutdown | ||
| -- began meanwhile, which merely restores pre-graceful-shutdown behavior | ||
| -- for bytes the client already sent. | ||
| mbs <- receiveNoWait sock pool | ||
| case mbs of | ||
| Just bs -> return bs | ||
| Nothing -> slowPath | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Doesn't this mean that we might never check the shutting down state if the client is prompt with their bytes and we happen to only |
||
| where | ||
| slowPath = do | ||
| sockWait <- | ||
| #if !WINDOWS && MIN_VERSION_network(3,2,2) | ||
| waitReadSocketSTM sock | ||
| waitReadSocketSTM sock | ||
| #else | ||
| -- FIXME: 'waitReadSocketSTM' doesn't work on WINDOWS, and actually | ||
| -- blocks indefinitely, so we fall back to going straight to 'recv'. | ||
| pure (pure ()) | ||
| -- FIXME: 'waitReadSocketSTM' doesn't work on WINDOWS, and actually | ||
| -- blocks indefinitely, so we fall back to going straight to 'recv'. | ||
| pure (pure ()) | ||
| #endif | ||
| isShuttingDown <- atomically $ | ||
| -- when shutting down | ||
| (checkShutdown $> True) | ||
| <|> | ||
| -- else wait for socket readiness and do non-blocking read | ||
| (sockWait $> False) | ||
| if isShuttingDown then pure "" else recv | ||
| where | ||
| isShuttingDown <- atomically $ | ||
| -- when shutting down | ||
| (checkShutdown $> True) | ||
| <|> | ||
| -- else wait for socket readiness and do non-blocking read | ||
| (sockWait $> False) | ||
| if isShuttingDown then pure "" else recv | ||
| recv = receive sock pool | ||
| checkShutdown = do | ||
| check =<< currentShuttingDownStateSTM ss | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't like the duplication of code here, though we might have to.
This will also need tests like 'withBufferPool', though, if it IS duplicated.
And it will need tests either way, even if we refactor
withBufferPoolfor this new functionality.