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

Exponentially reduce computation time needed for replace-dependency by u... #4313

Merged
merged 1 commit into from Sep 29, 2014

Conversation

roconnor
Copy link
Contributor

...sing memoization.

This patch makes two changes.

(1) It memoizes the computation of dependsOnOld.
(2) It replaces rewrittenDerivations with a similar memoized table rewriteMemo.

This prevents the entire tree of run-time dependencies from being traversed and instead only traverses the graph of run-time dependencies.
In the case of deep dependency changes (such as changing one's bash version for an entire NixOS system) this can lead to an exponential speedup in processing time
because shared dependencies are no longer traversed multiple times.

This patch isn't quite derivation-per-derivation equivalent to the original computation.
There are two immaterial differences.

(1) The previous version would always call upon sed to replace oldDependency with newDependency even when the store object being updated doesn't directly depend on
oldDependency.
The new version only replaceds oldDependency with newDependency when the store object being updated actually directly depends on oldDependency (which means there is
actually a hash to replace).
(2) The previous version would list the old store object as a source input of the new store object, except for the root derivation being updated. Because the
root derivation being updated has its actual derivation available the previous verions would make the updated root derivation depend on the old derivation as a
derivation input instead of as a source input.
The new version always lists the old store object as a source input, including the root derivation.

…y using memoization.

This patch makes two changes.

(1) It memoizes the computation of dependsOnOld.
(2) It replaces rewrittenDerivations with a similar memoized table rewriteMemo.

This prevents the entire tree of run-time dependencies from being traversed and instead only traverses the graph of run-time dependencies.
In the case of deep dependency changes (such as changing one's bash version for an entire NixOS system) this can lead to an exponential speedup in processing time
because shared dependencies are no longer traversed multiple times.

This patch isn't quite derivation-per-derivation equivalent to the original computation.
There are two immaterial differences.

(1) The previous version would always call upon sed to replace oldDependency with newDependency even when the store object being updated doesn't directly depend on
oldDependency.
The new version only replaceds oldDependency with newDependency when the store object being updated actually directly depends on oldDependency (which means there is
actually a hash to replace).
(2) The previous version would list the old store object as a source input of the new store object, *except* for the root derivation being updated.  Because the
root derivation being updated has its actual derivation avaiable the previous verions would make the updated root derivation depend on the old derivation as a
derivation input instead of a source input.
The new version always lists the old store object as a source input, including the root derivation.
@shlevy
Copy link
Member

shlevy commented Sep 28, 2014

@roconnor This looks right to me. I assume you tested it with the bash fix? If so and it works, let's merge it.

@wizeman
Copy link
Member

wizeman commented Sep 29, 2014

@roconnor I just tested this using replaceDependency on bash and it reduced my evaluation time from 65 minutes to almost instantaneously! Thanks!

Assuming it does the same thing as the old one, +1 for merging it.

roconnor added a commit that referenced this pull request Sep 29, 2014
Exponentially reduce computation time needed for replace-dependency by u...
@roconnor roconnor merged commit 6b2ecc5 into master Sep 29, 2014
@roconnor roconnor deleted the fastReplaceDeps branch September 29, 2014 01:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants