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

Wingman: Low gas warning #2038

Merged
merged 6 commits into from
Jul 29, 2021
Merged

Wingman: Low gas warning #2038

merged 6 commits into from
Jul 29, 2021

Conversation

isovector
Copy link
Collaborator

Make Wingman inform you if it ran out of gas when attempting to synthesize an expression, rather than just failing with a "couldn't be solved" error message.

Feature request from @masaeedu.

@isovector
Copy link
Collaborator Author

@pepeiborra the dynamically linked builds are now failing in CI with an undefined linker reference to my new constructor. Usually when I see this it means my cabal file's modules are not up to date, but that doesn't appear to be the case here. Any idea what's up?

@pepeiborra
Copy link
Collaborator

"Linking" the tactics test suite fails to find the symbol for the new constructor. I'm not sure what "linking" means here since we are asking Cabal to use dynamic linking for executables, it could be some sort of validation step, or TH if any.

We are doing dynamic linking with stack: this is probably not well trodden and involves telling cabal to link haskell packages dynamically (for executables) without stack being aware. What could be going wrong here?

If you have the time, please put together a minimal repro and send it upstream to stack or wherever it belongs.

In terms of fixing it, there's a couple of options:

  1. Disable the caching in CircleCI.
  2. Disable dynamic linking but find a way to reduce the memory footprint of linking, likely by switching to the gold linker.

I don't have time to investigate any further, hopefully you or any of the usual maintainers can take a look!

@isovector
Copy link
Collaborator Author

For anyone who might be reading the thread:

I'm not sure what "linking" means here since we are asking Cabal to use dynamic linking for executables

This is what appears in CI, and in my local builds:

hls-tactics-plugin         > Linking .stack-work/dist/x86_64-linux/Cabal-3.2.1.0/build/tests/tests ...
hls-tactics-plugin         > .stack-work/dist/x86_64-linux/Cabal-3.2.1.0/build/tests/tests-tmp/CodeAction/AutoSpec.dyn_o:function CodeActionziAutoSpec_spec1_info: error: undefined reference to 'hlszmtacticszmpluginzm1zi2zi0zi0zmFpMEBH6ApWkAbtR1G6wNS6_WingmanziTypes_NotEnoughGas_closure'
hls-tactics-plugin         > collect2: error: ld returned 1 exit status
hls-tactics-plugin         > `gcc' failed in phase `Linker'. (Exit code: 1)

@Ailrun
Copy link
Member

Ailrun commented Jul 26, 2021

Disable the caching in CircleCI.

IMO this is a non-option as it makes CircleCI be a serious bottleneck of PR processes. When the cache had a problem, CircleCI took about 5 hours, and we often have more than 3 somewhat active PRs, so the CI process will take about a day just by itself if we disable the cache.

@jneira
Copy link
Member

jneira commented Jul 27, 2021

Hmm i think the problem could be caused by .stack-work being cached. In theory local packages should not be cached but built from scratch for each run. The major part of saved time comes from caching dependencies, and that cache is in .stack
The dynamic linking has uncovered a stack bug i guess.

@isovector could you test locally if stack clean --full resolves the link problem?

@jneira
Copy link
Member

jneira commented Jul 27, 2021

Let see how much time cost build local packages from scratch: jneira#25

@jneira
Copy link
Member

jneira commented Jul 27, 2021

Ok, with the actual full cache (https://app.circleci.com/pipelines/github/jneira/haskell-language-server/642/workflows/0dfabe4c-3b34-49a3-8df6-e799103089db):

job id time
ghc-default 5425 24m 54s
ghc-9.0.1 5424 16m 35s
ghc-8.8.4 5429 25m 51s
ghc-8.8.3 5432 33m 49s
ghc-8.6.5 5426 20m 47s
ghc-8.6.4 5427 21m 32s
ghc-8.10.5 5430 24m 43s
ghc-8.10.4 5423 22m 13s
ghc-8.10.3 5431 27m 33s
ghc-8.10.2 5428 20m 56s

Caching only deps (https://app.circleci.com/pipelines/github/jneira/haskell-language-server/644/workflows/f66595fc-2e5e-46c5-b549-842e59340e84):

job id time
ghc-default 5446 27m 31s
ghc-9.0.1 5444 17m 4s
ghc-8.8.4 5447 37m 28s
ghc-8.8.3 5448 13m 21s
ghc-8.6.5 5449 20m 42s
ghc-8.6.4 5445 19m 10s
ghc-8.10.5 5451 1h 7m 30s (wout cache)
ghc-8.10.4 5450 15m 16s
ghc-8.10.3 5443 33m 14s
ghc-8.10.2 5452 12m 23s

Times are not uniform in the 2nd case so maybe it could be closed to full cache.

@pepeiborra
Copy link
Collaborator

pepeiborra commented Jul 27, 2021

The numbers look good @jneira, please send a PR to merge it

@isovector isovector requested review from Ailrun, pepeiborra and jneira and removed request for Ailrun July 28, 2021 17:32
@isovector isovector added the merge me Label to trigger pull request merge label Jul 28, 2021
Copy link
Member

@Ailrun Ailrun left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@jneira jneira left a comment

Choose a reason for hiding this comment

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

thanks for your phenomal work on wingman 🙂

@jneira
Copy link
Member

jneira commented Jul 29, 2021

Several jobs were stalled since 17 hours with the message:

Can't find any online and idle self-hosted or hosted runner in the current repository, account/organization that matches the required labels: 'ubuntu-latest'
Waiting for a self-hosted or a hosted runner to pickup this job...

As reported by @Ailrun in some pr i dont find out right now
I've cancelled all those jobs and will restart them in priority pr order (wingman ones first)
I've reported the error in gha: actions/runner#1230 (comment)

@jneira
Copy link
Member

jneira commented Jul 29, 2021

At least the testing workflow of this one has started to run 🤞

@mergify mergify bot merged commit 7a41ab7 into haskell:master Jul 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merge me Label to trigger pull request merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants