Skip to content

Commit

Permalink
Add an -O flag to turn on optimizations
Browse files Browse the repository at this point in the history
This will enable the optimizations even when debugging: see #229.
  • Loading branch information
jaspervdj-luminal authored Sep 23, 2020
1 parent d5bc4db commit fdad1b3
Show file tree
Hide file tree
Showing 15 changed files with 192 additions and 19 deletions.
10 changes: 10 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -344,6 +344,16 @@ here, you can do a number of things:
- Use [`:where`](#where) to see your current location.
- Use [`:quit`](#quit) to exit debugging mode. Use `:quit` again to exit the REPL.

#### Debugging notes

- By default, `fregot` turns off optimizations when debugging. This allows
you to more naturally follow what the code is doing. However, this may get
in your way when trying to debug complex queries that take too long to
execute without optimizations.

To explicitly turn on optimizations (even while debugging), use
`fregot repl -O`.

REPL Usage
----------

Expand Down
14 changes: 10 additions & 4 deletions lib/Fregot/Eval.hs
Original file line number Diff line number Diff line change
Expand Up @@ -675,19 +675,25 @@ evalStatement (AssignS source x y) = suspend source $ do
return muTrue
evalStatement (TermS e) = suspend (e ^. termAnn) (evalTerm e)
evalStatement (IndexedCompS source comp) = do
-- Evaluate the entire thing.
!index <- evalIndexedComprehension source comp
keyVals <- forM (comp ^. indexedKeys) (evalVar source >=> ground source)
let !collection = case HMS.lookup keyVals index of
cache <- view comprehensionCache
collection <- if cache ^. Cache.enabled then do
-- Evaluate the entire thing.
!index <- evalIndexedComprehension source comp
keyVals <- forM (comp ^. indexedKeys) (evalVar source >=> ground source)
pure $ case HMS.lookup keyVals index of
Just mu -> mu
Nothing -> muValueF $ case comp ^. indexedComprehension of
ArrayComp _ _ -> ArrayV V.empty
SetComp _ _ -> SetV HS.empty
ObjectComp _ _ _ -> ObjectV HMS.empty
else do
-- Cache not enabled, use normal evaluation.
evalComprehension source (comp ^. indexedComprehension)
iv <- toInstVar (comp ^. indexedAssignee)
_ <- unify source (Mu (FreeM iv)) collection
return muTrue


unify :: SourceSpan -> Mu' -> Mu' -> EvalM Mu'
unify _source (Mu WildcardM) y = return y
unify _source x (Mu WildcardM) = return x
Expand Down
2 changes: 1 addition & 1 deletion lib/Fregot/Eval/Cache.hs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
{-# LANGUAGE TemplateHaskell #-}
module Fregot.Eval.Cache
( Version
, Cache
, Cache, enabled
, new
, bump
, disable
Expand Down
32 changes: 23 additions & 9 deletions lib/Fregot/Interpreter.hs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
{-# LANGUAGE TypeFamilies #-}
module Fregot.Interpreter
( InterpreterM
, Config (..)
, defaultConfig
, Handle
, newHandle
, copyHandle
Expand Down Expand Up @@ -110,8 +112,16 @@ import System.FilePath.Extended (listExtensions)

type InterpreterM a = ParachuteT Error IO a

data Config = Config
{ _opt :: !Bool
, _dumpTags :: !Dump.Tags
}

defaultConfig :: Config
defaultConfig = Config True mempty

data Handle = Handle
{ _dumpTags :: !Dump.Tags
{ _config :: !Config
, _builtins :: !(IORef (Builtins Identity))
, _sources :: !Sources.Handle
-- | List of modules, and files we loaded them from. Grouped by package
Expand All @@ -124,13 +134,14 @@ data Handle = Handle
, _inputDoc :: !(IORef Eval.Value)
}

$(makeLenses ''Config)
$(makeLenses ''Handle)

newHandle
:: Dump.Tags
:: Config
-> Sources.Handle
-> IO Handle
newHandle _dumpTags _sources = do
newHandle _config _sources = do
initializedBuiltins <- traverse (htraverse (fmap Identity)) defaultBuiltins

_builtins <- IORef.newIORef initializedBuiltins
Expand All @@ -145,7 +156,7 @@ newHandle _dumpTags _sources = do
-- clears the cache.
copyHandle :: Handle -> IO Handle
copyHandle h = do
let _dumpTags = h ^. dumpTags
let _config = h ^. config
_sources <- copy (h ^. sources)
_builtins <- copy (h ^. builtins)
_modules <- copy (h ^. modules)
Expand Down Expand Up @@ -370,7 +381,7 @@ compileRules h = do

-- Compile the tree and save it.
tree1 <- mapParachuteT
(`runReaderT` (h ^. dumpTags))
(`runReaderT` (h ^. config . dumpTags))
(Compile.compileTree builtin tree0 preparedTree)
dieIfErrors
liftIO $ IORef.writeIORef (h ^. ruleTree) tree1
Expand Down Expand Up @@ -458,10 +469,7 @@ newResumeStep
-> InterpreterM (Eval.ResumeStep Eval.Value)
newResumeStep h pkgname query = do
-- Disable the cache for evaluating queries in resume steps.
envctx <-
over (Eval.ecEnvironment . Eval.ruleCache) Cache.disable .
over (Eval.ecEnvironment . Eval.comprehensionCache) Cache.disable <$>
newEvalContext h
envctx <- disableOpt <$> newEvalContext h
ctree <- liftIO $ IORef.readIORef (h ^. ruleTree)
universe <- universeForRenamer h

Expand All @@ -484,6 +492,12 @@ newResumeStep h pkgname query = do

dieIfErrors
return $ Eval.newResumeStep envctx (Eval.evalQuery cquery)
where
disableOpt
| h ^. config . opt = id
| otherwise =
over (Eval.ecEnvironment . Eval.ruleCache) Cache.disable .
over (Eval.ecEnvironment . Eval.comprehensionCache) Cache.disable

step
:: Handle -> Eval.ResumeStep Eval.Value
Expand Down
4 changes: 3 additions & 1 deletion lib/Fregot/Main/Bundle.hs
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,9 @@ parseOptions = Options
main :: GlobalOptions -> Options -> IO ExitCode
main gopts opts = do
sources <- Sources.newHandle
interpreter <- Interpreter.newHandle (gopts ^. dumpTags) sources
interpreter <- Interpreter.newHandle
(Interpreter.defaultConfig {Interpreter._dumpTags = gopts ^. dumpTags})
sources
regoPaths <- concat <$> traverse Find.findRegoFiles (opts ^. paths)
(errors, _) <- Parachute.runParachuteT $ do
forM_ regoPaths $ \path -> Interpreter.loadModule interpreter
Expand Down
4 changes: 3 additions & 1 deletion lib/Fregot/Main/Eval.hs
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,9 @@ parseOptions = Options
main :: GlobalOptions -> Options -> IO ExitCode
main gopts opts = do
sources <- Sources.newHandle
interpreter <- Interpreter.newHandle (gopts ^. dumpTags) sources
interpreter <- Interpreter.newHandle
(Interpreter.defaultConfig {Interpreter._dumpTags = gopts ^. dumpTags})
sources
regoPaths <- Find.findPrefixedRegoFiles (opts ^. paths)
(errors, mbResult) <- Parachute.runParachuteT $ do
forM_ regoPaths $ Interpreter.loadFileByExtension
Expand Down
12 changes: 11 additions & 1 deletion lib/Fregot/Main/Repl.hs
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ data Options = Options
{ _input :: Maybe FilePath
, _watch :: Bool
, _noHistoryFile :: Bool
, _opt :: Bool
, _paths :: [DestinationPrefix FilePath]
} deriving (Show)

Expand All @@ -45,14 +46,23 @@ parseOptions = Options
OA.long "no-history-file" <>
OA.hidden <>
OA.help "Don't save repl history to a file")
<*> (OA.switch $
OA.short 'O' <>
OA.hidden <>
OA.help "Turn on optimizations when debugging")
<*> (OA.many $ fmap parseDestinationPrefix $ OA.strArgument $
OA.metavar "PATHS" <>
OA.help "Rego files or directories to load into repl")

main :: GlobalOptions -> Options -> IO ExitCode
main gopts opts = do
sources <- Sources.newHandle
itpr <- Interpreter.newHandle (gopts ^. dumpTags) sources
itpr <- Interpreter.newHandle
(Interpreter.defaultConfig
{ Interpreter._opt = opts ^. opt
, Interpreter._dumpTags = gopts ^. dumpTags
})
sources
regoPaths <- Find.findPrefixedRegoFiles (opts ^. paths)

replConfig <-
Expand Down
4 changes: 3 additions & 1 deletion lib/Fregot/Main/Test.hs
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,9 @@ parseOptions = Options
main :: GlobalOptions -> Options -> IO ExitCode
main gopts opts = do
sources <- Sources.newHandle
interpreter <- Interpreter.newHandle (gopts ^. dumpTags) sources
interpreter <- Interpreter.newHandle
(Interpreter.defaultConfig {Interpreter._dumpTags = gopts ^. dumpTags})
sources
regoPaths <- Find.findPrefixedRegoFiles (opts ^. paths)
(errors, mbResult) <- Parachute.runParachuteT $ do
forM_ regoPaths $ Interpreter.loadFileByExtension
Expand Down
29 changes: 29 additions & 0 deletions tests/golden/repl/debug-noopt.goldplate
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
{
"command": "fregot",
"arguments": ["repl", "--no-history-file"],
"notes": [
"This actually takes fewer steps than `debug-opt` since we don't fully",
"calculate the indexed comprehension."
],
"asserts": [
{"exit_code": 0},
{"stderr": "${GOLDPLATE_NAME}.stderr"},
{"stdout": "${GOLDPLATE_NAME}.stdout"}
],
"stdin": [
":load ../opt/comprehension-index.rego",
":break exposed_ports_by_interface",
"access_twice {exposed_ports_by_interface[\"eth0\"]; exposed_ports_by_interface[\"lo1\"]}",
"access_twice",
":next",
":next",
"mock_input.exposed[j].port",
":next",
"mock_input.exposed[j].port",
":next",
":next",
":next",
":next",
":next"
]
}
9 changes: 9 additions & 0 deletions tests/golden/repl/debug-noopt.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
F u g u e R E G O T o o l k i t
fregot v0.12.3 repl - use :help for usage info
Loading ../opt/comprehension-index.rego...
Loaded package comprehension_index
Set breakpoint at comprehension_index.exposed_ports_by_interface
Rule access_twice added
Not paused
Not paused
Not paused
21 changes: 21 additions & 0 deletions tests/golden/repl/debug-noopt.stdout
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
repl% comprehension_index% comprehension_index% comprehension_index% 32| intf := mock_input.exposed[i].interface
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
comprehension_index(debug)% 35| mock_input.exposed[j].interface = intf
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
comprehension_index(debug)% 36| port := mock_input.exposed[j].port
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
comprehension_index(debug)% = 8080
| j = 0
| i = 0
| intf = "eth0"
comprehension_index(debug)% 36| port := mock_input.exposed[j].port
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
comprehension_index(debug)% = 8081
| j = 1
| i = 0
| intf = "eth0"
comprehension_index(debug)% 1| access_twice {exposed_ports_by_interface["eth0"]; exposed_ports_by_interface["lo1"]}
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
comprehension_index(debug)% (debug) = true
(debug) finished
comprehension_index% comprehension_index% comprehension_index% comprehension_index%
31 changes: 31 additions & 0 deletions tests/golden/repl/debug-opt.goldplate
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
{
"notes": [
"This actually takes more steps than `debug-noopt` since we fully",
"calculate the indexed comprehension! But that is what we're testing."
],
"command": "fregot",
"arguments": ["repl", "-O", "--no-history-file"],
"asserts": [
{"exit_code": 0},
{"stderr": "${GOLDPLATE_NAME}.stderr"},
{"stdout": "${GOLDPLATE_NAME}.stdout"}
],
"stdin": [
":load ../opt/comprehension-index.rego",
":break exposed_ports_by_interface",
"access_twice {exposed_ports_by_interface[\"eth0\"]; exposed_ports_by_interface[\"lo1\"]}",
"access_twice",
":next",
":next",
"mock_input.exposed[j].port",
":next",
"mock_input.exposed[j].port",
":next",
"mock_input.exposed[j].port",
":next",
"mock_input.exposed[j].port",
":next",
":next",
":next"
]
}
7 changes: 7 additions & 0 deletions tests/golden/repl/debug-opt.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
F u g u e R E G O T o o l k i t
fregot v0.12.3 repl - use :help for usage info
Loading ../opt/comprehension-index.rego...
Loaded package comprehension_index
Set breakpoint at comprehension_index.exposed_ports_by_interface
Rule access_twice added
Not paused
29 changes: 29 additions & 0 deletions tests/golden/repl/debug-opt.stdout
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
repl% comprehension_index% comprehension_index% comprehension_index% 32| intf := mock_input.exposed[i].interface
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
comprehension_index(debug)% 35| mock_input.exposed[j].interface = intf
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
comprehension_index(debug)% 36| port := mock_input.exposed[j].port
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
comprehension_index(debug)% = 8080
| j = 0
| intf = "eth0"
comprehension_index(debug)% 36| port := mock_input.exposed[j].port
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
comprehension_index(debug)% = 8081
| j = 1
| intf = "eth0"
comprehension_index(debug)% 36| port := mock_input.exposed[j].port
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
comprehension_index(debug)% = 443
| j = 2
| intf = "eth1"
comprehension_index(debug)% 36| port := mock_input.exposed[j].port
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
comprehension_index(debug)% = 5000
| j = 3
| intf = "lo1"
comprehension_index(debug)% 1| access_twice {exposed_ports_by_interface["eth0"]; exposed_ports_by_interface["lo1"]}
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
comprehension_index(debug)% (debug) = true
(debug) finished
comprehension_index% comprehension_index%
3 changes: 2 additions & 1 deletion tests/hs/Fregot/Interpreter/Tests.hs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,8 @@ tests :: Tasty.TestTree
tests = Tasty.testGroup "Fregot.Interpreter.Tests"
[ Tasty.testCase "insertBuiltin" $ do
sources <- Sources.newHandle
interpreter <- Interpreter.newHandle mempty sources
interpreter <- Interpreter.newHandle
Interpreter.defaultConfig sources
(errs, mbResult) <- runParachuteT $ do
Interpreter.insertBuiltin interpreter magicName magicImpl
let input = "test.magic()"
Expand Down

0 comments on commit fdad1b3

Please sign in to comment.