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

Optimize performance during uncached pod installation. #12154

Merged
merged 1 commit into from Nov 20, 2023

Conversation

dnkoutso
Copy link
Contributor

When installing pods from a given repo, we are currently over-doing a bunch of unnecessary work by copying the original repo from within a temp dir N times as many pods we are installing from that repo. We then trim (clean) the copied repo to the files that are only relevant to the pod we are installing.

This can be very slow if a repo is large and offers multiple podspecs.

We also need to do this work only if there is a prepare command to run which currently operates or can operate against the initial cloned repo.

The PR here optimizes by splitting pods that require a prepare_command and ones that do not. For the ones that need one we execute the existing behavior but for the ones that do not, we trim the cloned repo and then copy exactly the files we need.

This can speed up installation dramatically for repos that publish multiple podspecs.

@@ -15,7 +15,7 @@ def initialize(root, specs_by_platform)
# @return [void]
#
def clean!
clean_paths.each { |path| FileUtils.rm_rf(path) } if root.exist?
FileUtils.rm_rf(clean_paths) if root.exist?
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

optimization

@@ -51,8 +51,7 @@ def self.download(
if target && result.location && target != result.location
UI.message "Copying #{request.name} from `#{result.location}` to #{UI.path target}", '> ' do
Cache.read_lock(result.location) do
FileUtils.rm_rf target
FileUtils.cp_r(result.location, target)
FileUtils.cp_r(result.location, target, :remove_destination => true)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

minor optimization

@dnkoutso
Copy link
Contributor Author

@paulb777 I checked Firebase can be installed which includes a prepare_command with this change.

@dnkoutso dnkoutso force-pushed the optimize_pod_dir_cleaning branch 3 times, most recently from a3fb18f to 93c35d6 Compare November 19, 2023 17:48
@dnkoutso dnkoutso force-pushed the optimize_pod_dir_cleaning branch 4 times, most recently from 63d1347 to add1460 Compare November 19, 2023 19:23
@dnkoutso dnkoutso marked this pull request as ready for review November 19, 2023 19:41
@dnkoutso dnkoutso changed the title Optimize pod cache during installation Optimize performance during uncached pod installation. Nov 19, 2023
@dnkoutso dnkoutso requested a review from amorde November 19, 2023 21:31
Copy link
Contributor

@justinseanmartin justinseanmartin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any specs for the new code path? Or are there already cases that cover this?

lib/cocoapods/downloader/cache.rb Outdated Show resolved Hide resolved
@dnkoutso dnkoutso force-pushed the optimize_pod_dir_cleaning branch 7 times, most recently from 05ab212 to 940242c Compare November 20, 2023 14:15
@dnkoutso dnkoutso added this to the 1.15.0 milestone Nov 20, 2023
@dnkoutso
Copy link
Contributor Author

Any specs for the new code path? Or are there already cases that cover this?

we could add Firebase with a prepare command as an example to build and test.

Copy link
Member

@paulb777 paulb777 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Any measurements of the speedup that you can share?

lib/cocoapods/downloader/cache.rb Outdated Show resolved Hide resolved
@dnkoutso
Copy link
Contributor Author

Thanks! Any measurements of the speedup that you can share?

@paulb internally in a repo of ours that basically drove this change, we now see something that took a minute or two down to 15-20 seconds!

@dnkoutso
Copy link
Contributor Author

Our internal repo basically offers 10s of pods and the repo has gotten larger which is now much faster to setup and correctly.

@dnkoutso dnkoutso merged commit 61ecb41 into master Nov 20, 2023
5 checks passed
@dnkoutso dnkoutso deleted the optimize_pod_dir_cleaning branch November 20, 2023 15:41
@jerrymarino
Copy link

@dnkoutso @justinseanmartin this change is busting cocoapods for me in some apps. Here's a gist of what I ran into :

  1. checkout a repo with a podfile.lock on version N of a pod
  2. update the pod to version N + 1
  3. update cocopoads to this version
  4. run pod install
  5. the sources for the pod at hand are missing in the Pods directory

@dnkoutso
Copy link
Contributor Author

Will take a look.

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