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 globbing in PathList to speed up pod install #3699

Merged

Conversation

vincentisambart
Copy link
Contributor

I'm not sure if caching paths_for_attribute is wise or not, but in my test it makes pod install on our app much faster: 140 sec → 20 sec. All specs still pass.

It might be problematic if the content on the file system changes during the run, that's why it's more of an experiment that anything else.

What do you think?

@segiddins
Copy link
Member

Can we just invalidate when refreshing from the FS? Otherwise, this seems pretty good to me ;)

@vincentisambart
Copy link
Contributor Author

In fact that's the main problem: I don't see what would be the best place in the code to invalidate the cache.

@segiddins
Copy link
Member

Ah, that's in PathList. Maybe the caching would best belong there? (in relative_glob?)

@vincentisambart
Copy link
Contributor Author

Something like that new commit I just pushed?

(I know I still need to refactor relative_glob because Rubocop complains it's too complex)

@segiddins
Copy link
Member

Yup, something just like that!

@vincentisambart
Copy link
Contributor Author

Do you have an idea why here the code checks if the pattern is a string or not? The documentation of the method says it's supposed to be strings.

@segiddins
Copy link
Member

Nope, no clue. That can probably be removed.

@vincentisambart
Copy link
Contributor Author

Seeing how it's been used it seems it would be for Regexps, but with the specs now being in JSON I guess that doesn't make sense anymore.

The method signature says that patterns are supposed to be strings, and
as now specs are stored in JSON, regexps should not be used anymore.
@vincentisambart
Copy link
Contributor Author

I added a CHANGELOG entry and removed handling of regexp patterns.

@vincentisambart vincentisambart changed the title Cache paths_for_attribute to speed up pod install Cache globbing in PathList to speed up pod install Jun 19, 2015
neonichu added a commit that referenced this pull request Jun 21, 2015
…ibute

Cache globbing in PathList to speed up pod install
@neonichu neonichu merged commit 78c6f0e into CocoaPods:master Jun 21, 2015
@neonichu
Copy link
Member

Great, let's merge this! Thanks a lot, @vincentisambart 🎉

@vincentisambart vincentisambart deleted the vi-cache-paths_for_attribute branch June 22, 2015 23:12
@alloy
Copy link
Member

alloy commented Jul 7, 2015

@vincentisambart Nice work!

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.

None yet

4 participants