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

Faster 'cabal update' #3

Closed
wants to merge 2 commits into from
Closed

Conversation

tanakh
Copy link

@tanakh tanakh commented Apr 27, 2012

Currently, 'cabal update' uses Distribution.Simple.Utils.writeFileAtomic.
But it requires data as String, so very very slow.
3.7MB of 00-index.tar.gz is extracted to 61MB of 00-index.tar.
It takes about 8 seconds to write 00-index.tar on my machine (i5-2400 CPU @ 3.10GHz).
cabal-install can depends on bytestring', so I rewritewriteFileAtomic` to use bytstring.
The bottleneck is resolved (< 1 second).

@kosmikus
Copy link
Contributor

Thanks for the pull request. I'll try to look at it soon.

As a general policy, I think it would be best to submit pull requests to master, not release branches such as cabal-1.14 (except if they concern specific bugfixes in a release branch). We can then always decide to merge changes from master back into cabal-1.14.

@tanakh
Copy link
Author

tanakh commented Apr 27, 2012

Sorry for wrong way. On next time, I do so.

@dcoutts
Copy link
Contributor

dcoutts commented May 10, 2012

Looks ok to me. Only suggestion is that we should really move the new bytestring version of writeFileAtomic into the Cabal lib's Utils module.

@tanakh
Copy link
Author

tanakh commented May 14, 2012

I think so, too. Is there no reason that Cabal avoid to depend to bytestring?

@dcoutts
Copy link
Contributor

dcoutts commented Jul 2, 2012

Historically Cabal did not use ByteString because it did not work with hugs and nhc98. We don't worry about that anymore. In future we will want to make use of bytestring and text packages.

@tanakh
Copy link
Author

tanakh commented Jul 3, 2012

I understand. I will look forward to that time.

@tibbe
Copy link
Member

tibbe commented Aug 11, 2012

Merged to master as 85ad9f8.

@tibbe tibbe closed this Aug 11, 2012
merijn pushed a commit to merijn/cabal that referenced this pull request Apr 5, 2018
Add --eta-patches-directory option to epm install
@emilypi emilypi mentioned this pull request Aug 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants