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

Cache vsn information during the run to avoid extra unnecessary shell calls #177

Closed
wants to merge 1 commit into from
Closed

Conversation

yrashk
Copy link
Contributor

@yrashk yrashk commented Jan 19, 2012

My experiments showed that I can save a lot of time compiling a complex project by caching vsn information.

Here's the original up-to-date rebar timing (no compilation incurred, everything is up-to-date):

11.26s user 1.77s system 106% cpu 12.277 total

And here's after this patch:

6.47s user 0.78s system 113% cpu 6.364 total

Please consider merging it in.

@jlouis
Copy link

jlouis commented Jan 19, 2012

I don't know enough about rebar, but given the compile speedup, I'd like to see this get in.

@DeadZen
Copy link
Contributor

DeadZen commented Jan 19, 2012

+1

@Licenser
Copy link

+42

@ghost
Copy link

ghost commented Feb 1, 2012

For reference, what's the timed rebar invocation? Just rebar compile?

It may be a far-fetched use case, but won't the patch break
rebar compile update-deps compile
?

@yrashk
Copy link
Contributor Author

yrashk commented Feb 1, 2012

Yes, just rebar compile. The patch is likely to break that edge case, yes, unless we reinitialize the cache table in update-deps... or just before every command.

@yrashk
Copy link
Contributor Author

yrashk commented Feb 1, 2012

WDYT? Should we wipe out the table before each command?

@ghost
Copy link

ghost commented Feb 1, 2012

Not sure this is the right approach. Have to think.
Do you have many deps or deps with deep history where git describe is not instant?
How many sub directories have to be processed by rebar?
rebar_deps:preprocess/2 calling update_deps_code_path/1 may be the biggest culprit.

@yrashk
Copy link
Contributor Author

yrashk commented Feb 1, 2012

I have quite a lot of deps, and as you can see, caching git describe saves me a lot of time even on a dry run

@ghost
Copy link

ghost commented Feb 2, 2012

Let's revise the patch to wipe out the table before each command and get it merged.

@yrashk
Copy link
Contributor Author

yrashk commented Feb 2, 2012

@Tuncer updated

@ghost
Copy link

ghost commented Feb 2, 2012

Thanks, merged.

@ghost ghost closed this Feb 2, 2012
This pull request was closed.
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