Skip to content
This repository has been archived by the owner on Jan 2, 2021. It is now read-only.

Populate ms_hs_date in GetModSummary rule #694

Merged
merged 5 commits into from
Jul 22, 2020
Merged

Populate ms_hs_date in GetModSummary rule #694

merged 5 commits into from
Jul 22, 2020

Conversation

pepeiborra
Copy link
Collaborator

This is a straightforward fix that came up during haskell/haskell-language-server#191

@digitalasset-cla
Copy link

digitalasset-cla commented Jul 12, 2020 •
edited
Loading

CLA assistant check
All committers have signed the CLA.

@pepeiborra pepeiborra force-pushed the getmodsummary-modtime branch 2 times, most recently from 9afbbd6 to 42936ba Compare July 13, 2020 18:09
@pepeiborra pepeiborra force-pushed the getmodsummary-modtime branch from 42936ba to 91680da Compare July 18, 2020 19:20
@pepeiborra pepeiborra marked this pull request as draft July 18, 2020 19:29
@pepeiborra pepeiborra force-pushed the getmodsummary-modtime branch 2 times, most recently from 1ceabcf to 698ab6b Compare July 19, 2020 11:09
@pepeiborra
Copy link
Collaborator Author

I extended this change in the following directions:

  1. Platform dependent code to convert FileVersion to Maybe UTCTime. Further improvements possible in Mingw platforms to avoid UTCTime -> FileVersion -> Maybe UTCTime
  2. Add timestamps to the ModSummary code path for parsing: we need both parsing and interfaces mod summaries to have timestamps
  3. Added a new Shake rule ModSummaryWithoutTimestamp to preserve the existing early cutoff semantics and avoid a perf regression.

I tested the changes with the Eval plugin in haskell-language-server, branch: https://github.com/pepeiborra/ide/tree/eval

Please take another look and let me know

@pepeiborra pepeiborra marked this pull request as ready for review July 19, 2020 11:18
@pepeiborra
Copy link
Collaborator Author

pepeiborra commented Jul 19, 2020 •
edited
Loading

Benchmarks showing no perf regressions (in non-mingw): https://github.com/pepeiborra/ghcide/tree/getmodsummary-modtime-benchmarks/bench-hist

version name experiment maxResidency startup setup
upstream code actions 0.9317128790000001 209MB 10.373322118 1.151683226
HEAD code actions 0.8851059400000001 210MB 10.62859897 1.1537126960000001
upstream code actions after edit 30.920481852 209MB 9.899097365000001 0.0
HEAD code actions after edit 31.179141626000003 209MB 10.358541327000001 0.0
upstream completions after edit 8.112460716000001 171MB 10.121257906 0.0
HEAD completions after edit 8.267294747000001 179MB 10.808873808000001 0.0
upstream documentSymbols after edit 3.065205062 172MB 10.184797083000001 0.0
HEAD documentSymbols after edit 2.86471839 165MB 11.014139748 0.0
upstream edit 47.011598097000004 181MB 12.018539653000001 0.0
HEAD edit 46.720616624 181MB 10.239523371 0.0
upstream getDefinition 0.957393601 172MB 9.946836806 0.0
HEAD getDefinition 0.9199532 177MB 10.826650862000001 0.0
upstream hover 0.569961916 172MB 11.371499782 0.0
HEAD hover 0.24769026000000002 173MB 10.040305052 0.0
upstream hover after edit 1.5125140590000001 171MB 10.11517897 0.0
HEAD hover after edit 1.533573115 180MB 10.872858544000001 0.0

@pepeiborra pepeiborra force-pushed the getmodsummary-modtime branch from 698ab6b to 29bb521 Compare July 19, 2020 11:46
cocreature
cocreature approved these changes Jul 20, 2020
View reviewed changes
Copy link
Collaborator

@cocreature cocreature left a comment

Choose a reason for hiding this comment

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

Thanks! Looks good to me with two small comments.

src/Development/IDE/Core/Rules.hs Outdated Show resolved Hide resolved
src/Development/IDE/Core/Rules.hs Outdated Show resolved Hide resolved
@pepeiborra pepeiborra force-pushed the getmodsummary-modtime branch from 29bb521 to a273b5d Compare July 20, 2020 20:21
cocreature
cocreature approved these changes Jul 22, 2020
View reviewed changes
Copy link
Collaborator

@cocreature cocreature left a comment

Choose a reason for hiding this comment

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

Thanks!

@cocreature cocreature merged commit 02177b1 into haskell:master Jul 22, 2020
pepeiborra added a commit to pepeiborra/ide that referenced this pull request Dec 29, 2020
@pepeiborra
Populate ms_hs_date in GetModSummary rule (haskell/ghcide#694)
408e690 
* Populate ms_hs_date in GetModSummary rule

* More faithful ModSummary timestamps

* More ModSummary timestamps

* Address duplication

* Remove a displaced comment
pepeiborra added a commit to pepeiborra/ide that referenced this pull request Dec 29, 2020
@pepeiborra
Populate ms_hs_date in GetModSummary rule (haskell/ghcide#694)
5dd5f22 
* Populate ms_hs_date in GetModSummary rule

* More faithful ModSummary timestamps

* More ModSummary timestamps

* Address duplication

* Remove a displaced comment
pepeiborra added a commit to pepeiborra/ide that referenced this pull request Dec 29, 2020
@pepeiborra
Populate ms_hs_date in GetModSummary rule (haskell/ghcide#694)
8920267 
* Populate ms_hs_date in GetModSummary rule

* More faithful ModSummary timestamps

* More ModSummary timestamps

* Address duplication

* Remove a displaced comment
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@cocreature cocreature cocreature approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@pepeiborra @digitalasset-cla @cocreature