Skip to content

Commit a0c3be9

Browse files
Gabriella439mergify[bot]
authored andcommitted
Fix import resolution performance regression (#1522)
* Fix import resolution performance regression Related to #1511 This fixes a performance regression introduced in #1159 where `newManager` was being called on every remote import. This fixes that by going back to caching the `Manager` created by the first request. This leads to *dramatic* performance improvements for import-rich packages (like the Prelude or `dhall-kubernetes`) on the first import. For example, here are the performance numbers for importing the Prelude for a cold cache before and after this change: Before: ``` $ XDG_CACHE_HOME=.cache time dhall hash <<< 'https://prelude.dhall-lang.org/package.dhall' sha256:99462c205117931c0919f155a6046aec140c70fb8876d208c7c77027ab19c2fa 64.10 real 10.83 user 2.73 sys ``` After: ``` $ XDG_CACHE_HOME=.cache2 time dhall hash <<< 'https://prelude.dhall-lang.org/package.dhall' sha256:99462c205117931c0919f155a6046aec140c70fb8876d208c7c77027ab19c2fa 4.39 real 0.49 user 0.15 sys ``` That's ~16x faster! The improvement for `dhall-kubernetes` is smaller, but still significant: Before: ``` $ XDG_CACHE_HOME=.cache3 time dhall hash <<< ~/proj/dhall-kubernetes-charts/stable/jenkins/index.dhall sha256:04ebd960f6af331c49c3ccaedb353ac8269032b54fe0a29bd167febcd7104d4f 833.59 real 145.36 user 36.16 sys After: ``` $ XDG_CACHE_HOME=.cache4 time dhall hash <<< ~/proj/dhall-kubernetes-charts/stable/jenkins/index.dhall sha256:04ebd960f6af331c49c3ccaedb353ac8269032b54fe0a29bd167febcd7104d4f 381.41 real 8.41 user 1.91 sys ``` ... or ~2-3x improvement. * Fix `-f-with-http` build * Remove unnecessary `CPP` ... as caught by @sjakobi
1 parent 6a160db commit a0c3be9

File tree

7 files changed

+78
-8
lines changed

7 files changed

+78
-8
lines changed

dhall/dhall.cabal

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -535,6 +535,7 @@ Library
535535
if flag(with-http)
536536
Other-Modules:
537537
Dhall.Import.HTTP
538+
Dhall.Import.Manager
538539

539540
GHC-Options: -Wall -fwarn-incomplete-uni-patterns
540541
Default-Language: Haskell2010

dhall/ghc-src/Dhall/Import/HTTP.hs

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,9 @@
22
{-# LANGUAGE OverloadedStrings #-}
33
{-# LANGUAGE RecordWildCards #-}
44

5-
module Dhall.Import.HTTP where
5+
module Dhall.Import.HTTP
6+
( fetchFromHttpUrl
7+
) where
68

79
import Control.Exception (Exception)
810
import Control.Monad.IO.Class (MonadIO(..))
@@ -169,17 +171,27 @@ renderPrettyHttpException url e = case e of
169171
<> show e' <> "\n"
170172
#endif
171173

172-
newManager :: IO Manager
174+
newManager :: StateT Status IO Manager
173175
newManager = do
174176
let settings = HTTP.tlsManagerSettings
175-
#ifdef MIN_VERSION_http_client
176177
#if MIN_VERSION_http_client(0,5,0)
177178
{ HTTP.managerResponseTimeout = HTTP.responseTimeoutMicro (30 * 1000 * 1000) } -- 30 seconds
178179
#else
179180
{ HTTP.managerResponseTimeout = Just (30 * 1000 * 1000) } -- 30 seconds
180181
#endif
181-
#endif
182-
HTTP.newManager settings
182+
183+
Status { _manager = oldManager, ..} <- State.get
184+
185+
case oldManager of
186+
Nothing -> do
187+
manager <- liftIO (HTTP.newManager settings)
188+
189+
State.put (Status { _manager = Just manager , ..})
190+
191+
return manager
192+
193+
Just manager -> do
194+
return manager
183195

184196
data NotCORSCompliant = NotCORSCompliant
185197
{ expectedOrigins :: [ByteString]
@@ -260,7 +272,7 @@ type HTTPHeader = Network.HTTP.Types.Header
260272

261273
fetchFromHttpUrl :: URL -> Maybe [HTTPHeader] -> StateT Status IO Text.Text
262274
fetchFromHttpUrl childURL mheaders = do
263-
manager <- liftIO $ newManager
275+
manager <- newManager
264276

265277
let childURLString = Text.unpack (renderURL childURL)
266278

dhall/ghc-src/Dhall/Import/Manager.hs

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
{-| Both the GHC and GHCJS implementations of "Dhall.Import.Manager" export a
2+
`Manager` type suitable for use within the "Dhall.Import" module
3+
4+
For the GHC implementation the `Manager` type is a real `Manager` from the
5+
@http-client@ package. For the GHCJS implementation the `Manager` type is
6+
a synonym for @`Data.Void.Void`@ since GHCJS does not use a `Manager` for
7+
HTTP requests.
8+
-}
9+
module Dhall.Import.Manager
10+
( -- * Manager
11+
Manager
12+
) where
13+
14+
import Network.HTTP.Client (Manager)

dhall/ghcjs-src/Dhall/Import/HTTP.hs

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,10 @@
11
{-# LANGUAGE OverloadedStrings #-}
22
{-# LANGUAGE RecordWildCards #-}
33

4-
module Dhall.Import.HTTP where
4+
module Dhall.Import.HTTP
5+
( fetchFromHttpUrl
6+
, Manager
7+
) where
58

69
import Control.Monad.IO.Class (MonadIO(..))
710
import Control.Monad.Trans.State.Strict (StateT)
@@ -12,10 +15,19 @@ import Data.Semigroup ((<>))
1215
import qualified Data.Text as Text
1316
import qualified JavaScript.XHR
1417

18+
import Data.Void (Void)
1519
import Dhall.Core (URL(..))
1620
import Dhall.URL (renderURL)
1721
import Dhall.Import.Types
1822

23+
{-| The GHCJS implementation does not require a `Manager`
24+
25+
The purpose of this synonym is so that "Dhall.Import.Types" can import a
26+
`Manager` type from "Dhall.Import.HTTP" that does the correct thing for
27+
both the GHC and GHCJS implementations
28+
-}
29+
type Manager = Void
30+
1931
fetchFromHttpUrl
2032
:: URL
2133
-> Maybe [(CI ByteString, ByteString)]
Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
{-| Both the GHC and GHCJS implementations of "Dhall.Import.Manager" export a
2+
`Manager` type suitable for use within the "Dhall.Import" module
3+
4+
For the GHC implementation the `Manager` type is a real `Manager` from the
5+
@http-client@ package. For the GHCJS implementation the `Manager` type is
6+
a synonym for @`Data.Void.Void`@ since GHCJS does not use a `Manager` for
7+
HTTP requests.
8+
-}
9+
module Dhall.Import.Manager
10+
( -- * Manager
11+
Manager
12+
) where
13+
14+
import Data.Void (Void)
15+
16+
-- | GHCJS does not use a `Manager`
17+
type Manager = Void

dhall/src/Dhall/Import.hs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -172,7 +172,7 @@ import Dhall.Syntax
172172
, chunkExprs
173173
)
174174
#ifdef WITH_HTTP
175-
import Dhall.Import.HTTP hiding (HTTPHeader)
175+
import Dhall.Import.HTTP
176176
#endif
177177
import Dhall.Import.Types
178178

dhall/src/Dhall/Import/Types.hs

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
1+
{-# LANGUAGE CPP #-}
12
{-# LANGUAGE OverloadedStrings #-}
23
{-# LANGUAGE RecordWildCards #-}
4+
35
{-# OPTIONS_GHC -Wall #-}
46

57
module Dhall.Import.Types where
@@ -25,6 +27,9 @@ import Dhall.Core
2527
, ReifiedNormalizer(..)
2628
, URL
2729
)
30+
#ifdef WITH_HTTP
31+
import Dhall.Import.Manager (Manager)
32+
#endif
2833
import Dhall.Parser (Src)
2934
import Lens.Family (LensLike')
3035
import System.FilePath (isRelative, splitDirectories)
@@ -75,6 +80,13 @@ data Status = Status
7580
-- ^ Cache of imported expressions with their node id in order to avoid
7681
-- importing the same expression twice with different values
7782

83+
#ifdef WITH_HTTP
84+
, _manager :: Maybe Manager
85+
#else
86+
, _manager :: Maybe Void
87+
#endif
88+
-- ^ Used to cache the `Manager` when making multiple requests
89+
7890
, _remote :: URL -> StateT Status IO Data.Text.Text
7991
-- ^ The remote resolver, fetches the content at the given URL.
8092

@@ -96,6 +108,8 @@ emptyStatusWith _remote rootDirectory = Status {..}
96108

97109
_cache = Map.empty
98110

111+
_manager = Nothing
112+
99113
_normalizer = Nothing
100114

101115
_startingContext = Dhall.Context.empty

0 commit comments

Comments
 (0)