Skip to content
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

Fix resultBuilt(dirty mechanism) in hls-graph #4238

Merged
merged 19 commits into from
Jun 3, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
e3458c7
clarify dirty in hls-graph
soulomoon May 17, 2024
717b5b3
fix comment
soulomoon May 17, 2024
cedea72
hls-graph add `compute` test
soulomoon May 17, 2024
e4debec
Merge branch 'master' into 4237-clarify-the-dirty-mechanism-of-hls-graph
soulomoon May 17, 2024
3d7371e
move test to better place
soulomoon May 17, 2024
8021357
Merge remote-tracking branch 'refs/remotes/upstream/4237-clarify-the-…
soulomoon May 17, 2024
cfa9bc5
add detailed test
soulomoon May 17, 2024
7aed262
Merge branch 'master' into 4237-clarify-the-dirty-mechanism-of-hls-graph
soulomoon May 18, 2024
357a1be
Merge branch 'master' into 4237-clarify-the-dirty-mechanism-of-hls-graph
soulomoon May 20, 2024
7326170
Merge branch 'master' into 4237-clarify-the-dirty-mechanism-of-hls-graph
soulomoon May 21, 2024
41f7cdc
Merge branch 'master' into 4237-clarify-the-dirty-mechanism-of-hls-graph
soulomoon May 25, 2024
57f29a2
Merge branch 'master' into 4237-clarify-the-dirty-mechanism-of-hls-graph
soulomoon May 27, 2024
37a2bda
Merge branch 'master' into 4237-clarify-the-dirty-mechanism-of-hls-graph
soulomoon May 28, 2024
ad49146
Merge branch 'master' into 4237-clarify-the-dirty-mechanism-of-hls-graph
soulomoon May 29, 2024
1f84057
Merge branch 'master' into 4237-clarify-the-dirty-mechanism-of-hls-graph
soulomoon May 31, 2024
f003b71
Merge branch 'master' into 4237-clarify-the-dirty-mechanism-of-hls-graph
soulomoon Jun 1, 2024
3fe97e0
fix comment
soulomoon Jun 1, 2024
b0492cd
Merge branch 'master' into 4237-clarify-the-dirty-mechanism-of-hls-graph
soulomoon Jun 3, 2024
eacb43f
Merge branch 'master' into 4237-clarify-the-dirty-mechanism-of-hls-graph
michaelpj Jun 3, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 17 additions & 5 deletions hls-graph/src/Development/IDE/Graph/Internal/Database.hs
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,9 @@ builder db@Database{..} stack keys = withRunInIO $ \(RunInIO run) -> do
waitAll
pure results


-- | isDirty
-- only dirty when it's build time is older than the changed time of one of its dependencies
isDirty :: Foldable t => Result -> t (a, Result) -> Bool
isDirty me = any (\(_,dep) -> resultBuilt me < resultChanged dep)

Expand Down Expand Up @@ -179,14 +182,23 @@ compute db@Database{..} stack key mode result = do
deps <- newIORef UnknownDeps
(execution, RunResult{..}) <-
duration $ runReaderT (fromAction act) $ SAction db deps stack
built <- readTVarIO databaseStep
curStep <- readTVarIO databaseStep
deps <- readIORef deps
let changed = if runChanged == ChangedRecomputeDiff then built else maybe built resultChanged result
built' = if runChanged /= ChangedNothing then built else changed
Copy link
Collaborator Author

@soulomoon soulomoon May 17, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

set the new build time to changed might update the build time to a older one

-- only update the deps when the rule ran with changes
let lastChanged = maybe curStep resultChanged result
let lastBuild = maybe curStep resultBuilt result
-- changed time would be slower
-- build time would be faster
let (changed, built) = case runChanged of
-- some thing changed
ChangedRecomputeDiff -> (curStep, curStep)
-- recomputed is the same
ChangedRecomputeSame -> (lastChanged, curStep)
-- nothing changed
ChangedNothing -> (lastChanged, lastBuild)
let -- only update the deps when the rule ran with changes
actualDeps = if runChanged /= ChangedNothing then deps else previousDeps
previousDeps= maybe UnknownDeps resultDeps result
let res = Result runValue built' changed built actualDeps execution runStore
let res = Result runValue built changed curStep actualDeps execution runStore
case getResultDepsDefault mempty actualDeps of
deps | not (nullKeySet deps)
&& runChanged /= ChangedNothing
Expand Down
1 change: 0 additions & 1 deletion hls-graph/src/Development/IDE/Graph/Internal/Types.hs
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,6 @@ instance NFData RunMode where rnf x = x `seq` ()
-- | How the output of a rule has changed.
data RunChanged
= ChangedNothing -- ^ Nothing has changed.
| ChangedStore -- ^ The stored value has changed, but in a way that should be considered identical (used rarely).
Copy link
Collaborator Author

@soulomoon soulomoon May 17, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do not think it is ever used or would be used, but we could add back if there is a need for this.

| ChangedRecomputeSame -- ^ I recomputed the value and it was the same.
| ChangedRecomputeDiff -- ^ I recomputed the value and it was different.
deriving (Eq,Show,Generic)
Expand Down
Loading