From 05ab2126ef21896878b6adfd5553e4cfc57264b9 Mon Sep 17 00:00:00 2001 From: Dimitris Koutsogiorgas Date: Fri, 17 Nov 2023 12:31:11 +0200 Subject: [PATCH] Optimize performance during uncached pod installation. --- CHANGELOG.md | 4 +- lib/cocoapods/downloader.rb | 3 +- lib/cocoapods/downloader/cache.rb | 83 ++++++++++++++++++++---- lib/cocoapods/sandbox/pod_dir_cleaner.rb | 6 +- spec/unit/downloader/cache_spec.rb | 4 +- 5 files changed, 79 insertions(+), 21 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5fcdf1c490..24705c40a8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,7 +8,9 @@ To install release candidates run `[sudo] gem install cocoapods --pre` ##### Enhancements -* None. +* Optimize performance during uncached pod installation. + [Dimitris Koutsogiorgas](https://github.com/dnkoutso) + [#12154](https://github.com/CocoaPods/CocoaPods/pull/12154) ##### Bug Fixes diff --git a/lib/cocoapods/downloader.rb b/lib/cocoapods/downloader.rb index c36619f30e..b6892b20e3 100644 --- a/lib/cocoapods/downloader.rb +++ b/lib/cocoapods/downloader.rb @@ -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) end end end diff --git a/lib/cocoapods/downloader/cache.rb b/lib/cocoapods/downloader/cache.rb index 5d990e46e2..251b8bbb71 100644 --- a/lib/cocoapods/downloader/cache.rb +++ b/lib/cocoapods/downloader/cache.rb @@ -235,13 +235,38 @@ def cached_spec(request) # was not found in the download cache. # def uncached_pod(request) - in_tmpdir do |target| - result, podspecs = download(request, target) + in_tmpdir do |tmp_dir| + result, podspecs = download(request, tmp_dir) result.location = nil - podspecs.each do |name, spec| + # Split by pods that require a prepare command or not to speed up installation. + no_prep_cmd_specs, prep_cmd_specs = podspecs.partition { |_, spec| spec.prepare_command.nil? }.map(&:to_h) + + # Pods with a prepare command currently copy the entire repo, run the prepare command against the whole + # repo and then clean it up. We configure those first to ensure the repo is pristine. + prep_cmd_specs.each do |name, spec| destination = path_for_pod(request, :name => name, :params => result.checkout_options) - copy_and_clean(target, destination, spec) + copy_source_and_clean(tmp_dir, destination, spec) + write_spec(spec, path_for_spec(request, :name => name, :params => result.checkout_options)) + if request.name == name + result.location = destination + end + end + + specs_by_platform = group_subspecs_by_platform(no_prep_cmd_specs.values) + + # Remaining pods without a prepare command can be optimized by cleaning the repo first + # and then copying only the files needed. + pod_dir_cleaner = Sandbox::PodDirCleaner.new(tmp_dir, specs_by_platform) + Cache.write_lock(tmp_dir) do + pod_dir_cleaner.clean! + end + + no_prep_cmd_specs.each do |name, spec| + destination = path_for_pod(request, :name => name, :params => result.checkout_options) + file_accessors = pod_dir_cleaner.file_accessors.select { |fa| fa.spec.root.name == spec.name } + files = Pod::Sandbox::FileAccessor.all_files(file_accessors).map(&:to_s) + copy_files(files, tmp_dir, destination) write_spec(spec, path_for_spec(request, :name => name, :params => result.checkout_options)) if request.name == name result.location = destination @@ -279,23 +304,55 @@ def in_tmpdir(&blk) # # @return [Void] # - def copy_and_clean(source, destination, spec) - specs_by_platform = group_subspecs_by_platform(spec) + def copy_source_and_clean(source, destination, spec) + specs_by_platform = group_subspecs_by_platform([spec]) destination.parent.mkpath Cache.write_lock(destination) do - FileUtils.rm_rf(destination) - FileUtils.cp_r(source, destination) + FileUtils.cp_r(source, destination, :remove_destination => true) Pod::Installer::PodSourcePreparer.new(spec, destination).prepare! Sandbox::PodDirCleaner.new(destination, specs_by_platform).clean! end end - def group_subspecs_by_platform(spec) + # Copies the `files` from the `source` directory to `destination` _without_ cleaning the + # `destination` directory of any files unused by `spec`. This is a faster version used when + # installing pods without a prepare command in which has already happened prior. + # + # @param [Array] files + # + # @param [Pathname] source + # + # @param [Pathname] destination + # + # @return [Void] + # + def copy_files(files, source, destination) + files = files.select { |f| File.exist?(f) } + destination.mkpath + Cache.write_lock(destination) do + FileUtils.rm_rf(destination) + destination.mkpath + files_by_dir = files.group_by do |file| + relative_path = Pathname(file).relative_path_from(Pathname(source)).to_s + destination_path = File.join(destination, relative_path) + File.dirname(destination_path) + end + + files_by_dir.each do |dir, files_to_copy| + FileUtils.mkdir_p(dir) + FileUtils.cp_r(files_to_copy, dir) + end + end + end + + def group_subspecs_by_platform(specs) specs_by_platform = {} - [spec, *spec.recursive_subspecs].each do |ss| - ss.available_platforms.each do |platform| - specs_by_platform[platform] ||= [] - specs_by_platform[platform] << ss + specs.each do |spec| + [spec, *spec.recursive_subspecs].each do |ss| + ss.available_platforms.each do |platform| + specs_by_platform[platform] ||= [] + specs_by_platform[platform] << ss + end end end specs_by_platform diff --git a/lib/cocoapods/sandbox/pod_dir_cleaner.rb b/lib/cocoapods/sandbox/pod_dir_cleaner.rb index c47cf96489..fe152990c6 100644 --- a/lib/cocoapods/sandbox/pod_dir_cleaner.rb +++ b/lib/cocoapods/sandbox/pod_dir_cleaner.rb @@ -15,11 +15,9 @@ 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? end - private - # @return [Array] the file accessors for all the # specifications on their respective platform. # @@ -29,6 +27,8 @@ def file_accessors end end + private + # @return [Sandbox::PathList] The path list for this Pod. # def path_list diff --git a/spec/unit/downloader/cache_spec.rb b/spec/unit/downloader/cache_spec.rb index b39ff2501f..e702108ac8 100644 --- a/spec/unit/downloader/cache_spec.rb +++ b/spec/unit/downloader/cache_spec.rb @@ -49,7 +49,7 @@ module Pod end end - @cache.send(:group_subspecs_by_platform, @spec).should == { + @cache.send(:group_subspecs_by_platform, [@spec]).should == { Platform.new(:ios, '8.0') => [@spec.subspecs.first], Platform.new(:ios, '6.0') => [@spec], Platform.new(:osx, '10.7') => [@spec], @@ -81,7 +81,7 @@ module Pod end it 'downloads the source' do - @cache.expects(:copy_and_clean).twice + @cache.expects(:copy_files).twice response = @cache.download_pod(@unreleased_request) response.should == Downloader::Response.new(@cache.root + @unreleased_request.slug, @spec, @spec.source) end