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

No-op "spago build" takes too long #110

Closed
dwhitney opened this issue Feb 12, 2019 · 11 comments
Closed

No-op "spago build" takes too long #110

dwhitney opened this issue Feb 12, 2019 · 11 comments

Comments

@dwhitney
Copy link

It appears the latest version of spago merges build and install commands. This slows down the build a bit, and I'm not sure I like it. What do you think about either going back to manual install and build or having an alternate command that merges build and install?

@f-f
Copy link
Member

f-f commented Feb 12, 2019

@dwhitney yes, this change is due to #75

Basically what would happen to many (me included) is that they'd forget to run install before build. Also it makes sense that if the packages are not there the tool would try to install them instead of failing helplessly with a compiler error due to "modules not found".

So I'd like to have this feature in as it's good UX, but we should look at if it can be implemented in a more performant way - and I believe it can, since now we do it pretty naively (note: psc-package implements this in the same naive way).

@dwhitney how much of a slowdown do you notice? Could you try benchmarking on your project with 0.6.3 (which doesn't have this feature) vs 0.6.4?

@dwhitney
Copy link
Author

A no-op compile takes about 6.5 seconds on my 60 dep project. It's the same on both, so I will close this issue. I guess I feel like a no-op should be under 1 seconds, but also I have no idea what's going on under the hood.

@justinwoo
Copy link
Contributor

Did you freeze the hashes in your packages and whatnot? I would guess that's why it's slow

@dwhitney
Copy link
Author

@justinwoo freeze? I don't see that in the docs. Do you mean get a sha for the upstream or not use a branch on one of my repos?

@f-f
Copy link
Member

f-f commented Feb 12, 2019

@dwhitney I just tried a no-op operation in acme-spago, which contains the entirety of the package-set (181 packages), and it takes 9 seconds, which is a bit suspicious.

I'd like to investigate to see if we can make it better, so I'll reopen this issue

@f-f f-f reopened this Feb 12, 2019
@f-f f-f changed the title "spago build" implicitly includes "spago install"? No-op "spago build" takes too long Feb 12, 2019
@f-f
Copy link
Member

f-f commented Feb 12, 2019

@dwhitney yeah the docs don't mention it because "freezing with an integrity check" is a Dhall feature: basically if you have the sha set on the upstream the package-set will be cached.

If you don't have the sha, you can get one by either:

  • running spago spacchetti-upgrade to switch to the latest Spacchetti (this will generate the sha for the new set)
  • running dhall freeze --inplace packages.dhall (note: you'll have to use dhall compiled from master since we had to get an unreleased fix in. You can use the binary I linked or build it with stack)

@f-f
Copy link
Member

f-f commented Feb 12, 2019

I'm now wondering if having spago freeze would be the best actually

@f-f
Copy link
Member

f-f commented Feb 13, 2019

Update: the purs compile command in acme-spago takes ~5.5s alone:

$ time purs compile  ".spago/aff/v5.0.2/src/**/*.purs" ".spago/aff-coroutines/v7.0.0/src/**/*.purs" ".spago/aff-promise/v2.0.1/src/**/*.purs" ".spago/affjax/v7.0.0/src/**/*.purs" ".spago/ansi/v5.0.0/src/**/*.purs" ".spago/argonaut/v5.0.0/src/**/*.purs" ".spago/argonaut-codecs/v5.1.0/src/**/*.purs" ".spago/argonaut-core/v4.0.1/src/**/*.purs" ".spago/argonaut-generic/v3.0.0/src/**/*.purs" ".spago/argonaut-traversals/v6.0.0/src/**/*.purs" ".spago/arraybuffer-types/v2.0.0/src/**/*.purs" ".spago/arrays/v5.1.1/src/**/*.purs" ".spago/assert/v4.0.0/src/**/*.purs" ".spago/avar/v3.0.0/src/**/*.purs" ".spago/behaviors/v7.0.0/src/**/*.purs" ".spago/bifunctors/v4.0.0/src/**/*.purs" ".spago/bigints/v4.0.0/src/**/*.purs" ".spago/canvas/v4.0.0/src/**/*.purs" ".spago/catenable-lists/v5.0.0/src/**/*.purs" ".spago/chirashi/v0.1.0/src/**/*.purs" ".spago/choco-pie/v3.0.0/src/**/*.purs" ".spago/colors/v5.0.0/src/**/*.purs" ".spago/console/v4.2.0/src/**/*.purs" ".spago/const/v4.1.0/src/**/*.purs" ".spago/contravariant/v4.0.0/src/**/*.purs" ".spago/control/v4.1.0/src/**/*.purs" ".spago/coroutines/v5.0.0/src/**/*.purs" ".spago/css/v4.0.0/src/**/*.purs" ".spago/datetime/v4.1.0/src/**/*.purs" ".spago/debug/v4.0.0/src/**/*.purs" ".spago/decimals/v4.0.0/src/**/*.purs" ".spago/distributive/v4.0.0/src/**/*.purs" ".spago/dom-indexed/v6.0.0/src/**/*.purs" ".spago/effect/v2.0.0/src/**/*.purs" ".spago/either/v4.1.1/src/**/*.purs" ".spago/email-validate/v4.0.0/src/**/*.purs" ".spago/enums/v4.0.0/src/**/*.purs" ".spago/errors/v4.0.1/src/**/*.purs" ".spago/event/v1.2.4/src/**/*.purs" ".spago/exceptions/v4.0.0/src/**/*.purs" ".spago/exists/v4.0.0/src/**/*.purs" ".spago/expect-inferred/v0.2.0/src/**/*.purs" ".spago/filterable/v3.0.1/src/**/*.purs" ".spago/fixed-points/v5.1.0/src/**/*.purs" ".spago/foldable-traversable/v4.1.1/src/**/*.purs" ".spago/foreign/v5.0.0/src/**/*.purs" ".spago/foreign-generic/v7.0.0/src/**/*.purs" ".spago/foreign-object/v1.1.0/src/**/*.purs" ".spago/fork/v4.0.0/src/**/*.purs" ".spago/form-urlencoded/v4.0.1/src/**/*.purs" ".spago/formatters/v4.0.0/src/**/*.purs" ".spago/free/v5.1.0/src/**/*.purs" ".spago/freeap/v5.0.1/src/**/*.purs" ".spago/freet/v4.0.0/src/**/*.purs" ".spago/functions/v4.0.0/src/**/*.purs" ".spago/functors/v3.1.1/src/**/*.purs" ".spago/gen/v2.1.0/src/**/*.purs" ".spago/generics-rep/v6.1.0/src/**/*.purs" ".spago/globals/v4.0.0/src/**/*.purs" ".spago/gomtang-basic/v0.2.0/src/**/*.purs" ".spago/group/v4.0.0/src/**/*.purs" ".spago/halogen/v4.0.0/src/**/*.purs" ".spago/halogen-bootstrap/v8.0.0/src/**/*.purs" ".spago/halogen-css/v8.0.0/src/**/*.purs" ".spago/halogen-renderless/v0.0.3/src/**/*.purs" ".spago/halogen-select/v3.0.1/src/**/*.purs" ".spago/halogen-vdom/v3.0.0/src/**/*.purs" ".spago/heterogeneous/v0.3.0/src/**/*.purs" ".spago/http-methods/v4.0.2/src/**/*.purs" ".spago/identity/v4.1.0/src/**/*.purs" ".spago/integers/v4.0.0/src/**/*.purs" ".spago/invariant/v4.1.0/src/**/*.purs" ".spago/jajanmen/v0.2.0/src/**/*.purs" ".spago/js-date/v6.0.0/src/**/*.purs" ".spago/js-timers/v4.0.1/src/**/*.purs" ".spago/kancho/v1.0.0/src/**/*.purs" ".spago/lazy/v4.0.0/src/**/*.purs" ".spago/lcg/v2.0.0/src/**/*.purs" ".spago/lenient-html-parser/v4.0.0/src/**/*.purs" ".spago/lists/v5.3.0/src/**/*.purs" ".spago/machines/v5.1.0/src/**/*.purs" ".spago/makkori/v1.0.0/src/**/*.purs" ".spago/math/v2.1.1/src/**/*.purs" ".spago/maybe/v4.0.1/src/**/*.purs" ".spago/media-types/v4.0.1/src/**/*.purs" ".spago/memoize/v5.0.0/src/**/*.purs" ".spago/milkis/v6.0.1/src/**/*.purs" ".spago/minibench/v2.0.0/src/**/*.purs" ".spago/mmorph/v5.1.0/src/**/*.purs" ".spago/naporitan/v0.2.0/src/**/*.purs" ".spago/newtype/v3.0.0/src/**/*.purs" ".spago/node-buffer/v5.0.0/src/**/*.purs" ".spago/node-child-process/v5.0.0/src/**/*.purs" ".spago/node-fs/v5.0.0/src/**/*.purs" ".spago/node-fs-aff/v6.0.0/src/**/*.purs" ".spago/node-he/v0.2.0/src/**/*.purs" ".spago/node-http/v5.0.0/src/**/*.purs" ".spago/node-path/v3.0.0/src/**/*.purs" ".spago/node-postgres/v5.0.0/src/**/*.purs" ".spago/node-process/v6.0.0/src/**/*.purs" ".spago/node-readline/v4.0.0/src/**/*.purs" ".spago/node-sqlite3/v6.0.0/src/**/*.purs" ".spago/node-streams/v4.0.0/src/**/*.purs" ".spago/node-telegram-bot-api/v4.0.0/src/**/*.purs" ".spago/node-url/v4.0.0/src/**/*.purs" ".spago/nonempty/v5.0.0/src/**/*.purs" ".spago/now/v4.0.0/src/**/*.purs" ".spago/nullable/v4.1.0/src/**/*.purs" ".spago/numbers/v6.0.0/src/**/*.purs" ".spago/options/v4.0.0/src/**/*.purs" ".spago/ordered-collections/v1.4.0/src/**/*.purs" ".spago/orders/v4.0.0/src/**/*.purs" ".spago/pairing/v5.1.0/src/**/*.purs" ".spago/parallel/v4.0.0/src/**/*.purs" ".spago/parsing/v5.0.2/src/**/*.purs" ".spago/partial/v2.0.0/src/**/*.purs" ".spago/phoenix/v4.0.0/src/**/*.purs" ".spago/pipes/v6.0.0/src/**/*.purs" ".spago/polyform/v0.7.0/src/**/*.purs" ".spago/posix-types/v4.0.0/src/**/*.purs" ".spago/prelude/v4.1.0/src/**/*.purs" ".spago/prettier/v0.2.0/src/**/*.purs" ".spago/profunctor/v4.0.0/src/**/*.purs" ".spago/profunctor-lenses/v5.0.0/src/**/*.purs" ".spago/proxy/v3.0.0/src/**/*.purs" ".spago/psci-support/v4.0.0/src/**/*.purs" ".spago/random/v4.0.0/src/**/*.purs" ".spago/rationals/v5.0.0/src/**/*.purs" ".spago/react/v6.1.0/src/**/*.purs" ".spago/react-basic/v6.0.0/src/**/*.purs" ".spago/react-dom/v6.0.0/src/**/*.purs" ".spago/record/v1.0.0/src/**/*.purs" ".spago/record-extra/v1.0.0/src/**/*.purs" ".spago/record-format/v0.1.0/src/**/*.purs" ".spago/redux-devtools/v0.1.0/src/**/*.purs" ".spago/refs/v4.1.0/src/**/*.purs" ".spago/routing/v8.0.0/src/**/*.purs" ".spago/run/v2.0.0/src/**/*.purs" ".spago/semirings/v5.0.0/src/**/*.purs" ".spago/shoronpo/v0.3.0/src/**/*.purs" ".spago/signal/v10.1.0/src/**/*.purs" ".spago/sijidou/v0.1.0/src/**/*.purs" ".spago/simple-json/v4.4.0/src/**/*.purs" ".spago/simple-json-generics/v0.1.0/src/**/*.purs" ".spago/smolder/v11.0.1/src/**/*.purs" ".spago/spec/v3.1.0/src/**/*.purs" ".spago/st/v4.0.0/src/**/*.purs" ".spago/string-parsers/v5.0.0/src/**/*.purs" ".spago/strings/v4.0.1/src/**/*.purs" ".spago/strongcheck/v4.1.1/src/**/*.purs" ".spago/sunde/v1.0.0/src/**/*.purs" ".spago/svg-parser-halogen/v0.4.0/src/**/*.purs" ".spago/tailrec/v4.0.0/src/**/*.purs" ".spago/these/v4.0.0/src/**/*.purs" ".spago/toppokki/v1.0.0/src/**/*.purs" ".spago/tortellini/v3.0.0/src/**/*.purs" ".spago/transformers/v4.1.0/src/**/*.purs" ".spago/tuples/v5.1.0/src/**/*.purs" ".spago/type-equality/v3.0.0/src/**/*.purs" ".spago/type-isequal/v0.1.0/src/**/*.purs" ".spago/typelevel/v4.0.0/src/**/*.purs" ".spago/typelevel-prelude/v3.0.0/src/**/*.purs" ".spago/unfoldable/v4.0.0/src/**/*.purs" ".spago/unicode/v4.0.1/src/**/*.purs" ".spago/unordered-collections/v1.6.0/src/**/*.purs" ".spago/unsafe-coerce/v4.0.0/src/**/*.purs" ".spago/unsafe-reference/v3.0.1/src/**/*.purs" ".spago/validation/v4.0.0/src/**/*.purs" ".spago/variant/v5.2.0/src/**/*.purs" ".spago/web-clipboard/v1.0.0/src/**/*.purs" ".spago/web-dom/v1.0.0/src/**/*.purs" ".spago/web-events/v1.0.1/src/**/*.purs" ".spago/web-file/v1.1.0/src/**/*.purs" ".spago/web-html/v1.2.0/src/**/*.purs" ".spago/web-socket/v1.0.0/src/**/*.purs" ".spago/web-storage/v2.0.0/src/**/*.purs" ".spago/web-touchevents/v1.0.0/src/**/*.purs" ".spago/web-uievents/v1.1.0/src/**/*.purs" ".spago/web-xhr/v2.0.0/src/**/*.purs" ".spago/xiaomian/v0.1.0/src/**/*.purs" ".spago/yargs/v4.0.0/src/**/*.purs" "src/**/*.purs" "test/**/*.purs"
purs compile        12.60s user 1.49s system 262% cpu 5.368 total

Given that it's not possible to get rid of this time (well we could keep track of "have the source paths been modified?", but it's a lot of work), this leaves us with 3.5s from a no-op install 🤔

@f-f
Copy link
Member

f-f commented Feb 14, 2019

Update:

  • it looks like there was some duplicated work being done while calling the install, so that can be cut away (shaves about 2s in acme-spago)
  • switching from Data.Map to Data.HashMap seems to save another ~0.5s
  • the remaining time is spent in getTransitiveDeps, and while it looks like there is some duplicated work there - because every branch in the "tree of dependencies" is processed separately, we end up calculating many times things like "which are the transitive dependencies of foreign?"
    However, I wasn't able to put together a faster implementation than the current one (I guess it could be solved with an MVar to share a mutable cache, or more functionally with something like a histo)

@dwhitney I'll put together a PR for this so you can test if this is any faster :)

@dwhitney
Copy link
Author

dwhitney commented Feb 14, 2019 via email

@f-f
Copy link
Member

f-f commented Feb 14, 2019

Fix is up in #112

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants