Avoid expensive version calculations during build plan construction

[sjakobi]

For no-op builds of the these package I see speedups of about 17%. For the stack project it's only around 8%.

[mgsloan]

Hmm, I think there's a misunderstanding here about runOnce. join . runOnce is essentially id. The action returned by runOnce lets us ask for the value. That action can be run multiple times, but the computation only gets run once (hence the name).

[sjakobi]

Oh, I realized that the runOnce wouldn't help and removed it in my second commit, but I hadn't thought that it was quite so bad!

I'll just squash my two commits and hope that the combination makes more sense.

[mgsloan]

Thanks for working on this, I think there's a bit more work to do here (see comment)

[sjakobi]

Thanks for working on this, I think there's a bit more work to do here (see comment)

Please take a look at the squashed commit.

[mgsloan]

Gotcha! LGTM!

The main point of #1892 is about avoiding loading the cache, not about the expensive computation. This is a good change too, though, so merging

[sjakobi]

Thanks for the review and the merge!

The main point of #1892 is about avoiding loading the cache, not about the expensive computation.

Do you agree with my point that the call to getPackageCaches in constructPlan is entirely harmless because withLoadPackage (or more specifically withCabalLoader) has already performed the deserialization and stored the caches in the IORef?

In that case I believe the core issue of #1892 is already resolved.

[mgsloan]

Ah, I see where some of the confusion around runOnce may have come from - bde3a4#commitcomment-17280506

I have made a change such that withCabalLoader only loads the cache if it actually needs to. The case we're targeting here is where the user hasn't changed any dependencies, and is just doing a rebuild where the local packages are either unmodified or changed in some way. In this case, we should not need the hackage index.