Merge pull request #7348 from CocoaPods/seg-perf

(Not so) Small performance improvements

CHANGELOG.md

@@ -63,6 +63,9 @@ To install release candidates run `[sudo] gem install cocoapods --pre`
   [Keith Smiley](https://github.com/keith)
   [Samuel Giddins](https://github.com/segiddins)
 
+* Speed up `pod install` times by up to 50% for very large project.  
+  [Samuel Giddins](https://github.com/segiddins)
+
 
 ## 1.4.0 (2018-01-18)
 

lib/cocoapods/installer/analyzer.rb

@@ -512,8 +512,9 @@ module Pod
             hash[name] = values.sort_by { |pt| pt.specs.count }
           end
           pod_targets.each do |target|
-            dependencies = transitive_dependencies_for_specs(target.specs.reject(&:test_specification?), target.platform, all_resolver_specs).group_by(&:root)
-            test_dependencies = transitive_dependencies_for_specs(target.specs.select(&:test_specification?), target.platform, all_resolver_specs).group_by(&:root)
+            all_specs = all_resolver_specs.to_set
+            dependencies = transitive_dependencies_for_specs(target.non_test_specs.to_set, target.platform, all_specs).group_by(&:root)
+            test_dependencies = transitive_dependencies_for_specs(target.test_specs.to_set, target.platform, all_specs).group_by(&:root)
             test_dependencies.delete_if { |k| dependencies.key? k }
             target.dependent_targets = filter_dependencies(dependencies, pod_targets_by_name, target)
             target.test_dependent_targets = filter_dependencies(test_dependencies, pod_targets_by_name, target)
@@ -527,8 +528,9 @@ module Pod
             end
 
             pod_targets.each do |target|
-              dependencies = transitive_dependencies_for_specs(target.specs.reject(&:test_specification?), target.platform, specs.map(&:spec)).group_by(&:root)
-              test_dependencies = transitive_dependencies_for_specs(target.specs.select(&:test_specification?), target.platform, specs.map(&:spec)).group_by(&:root)
+              all_specs = specs.map(&:spec).to_set
+              dependencies = transitive_dependencies_for_specs(target.non_test_specs.to_set, target.platform, all_specs).group_by(&:root)
+              test_dependencies = transitive_dependencies_for_specs(target.test_specs.to_set, target.platform, all_specs).group_by(&:root)
               test_dependencies.delete_if { |k| dependencies.key? k }
               target.dependent_targets = pod_targets.reject { |t| dependencies[t.root_spec].nil? }
               target.test_dependent_targets = pod_targets.reject { |t| test_dependencies[t.root_spec].nil? }
@@ -566,16 +568,22 @@ module Pod
       #
       def transitive_dependencies_for_specs(specs, platform, all_specs)
         return [] if specs.empty? || all_specs.empty?
-        dependent_specs = specs.flat_map do |spec|
-          spec.consumer(platform).dependencies.flat_map do |dependency|
-            all_specs.find do |s|
+
+        dependent_specs = Set.new
+        specs.each do |spec|
+          spec.consumer(platform).dependencies.each do |dependency|
+            match = all_specs.find do |s|
+              next false unless s.name == dependency.name
               next false if specs.include?(s)
-              s.name == dependency.name
+              true
             end
-          end.compact
-        end.uniq
+            dependent_specs << match if match
+          end
+        end
+
         remaining_specs = all_specs - dependent_specs
-        dependent_specs + transitive_dependencies_for_specs(dependent_specs, platform, remaining_specs)
+
+        dependent_specs.union transitive_dependencies_for_specs(dependent_specs, platform, remaining_specs)
       end
 
       # Create a target for each spec group

lib/cocoapods/installer/xcode/pods_project_generator/pod_target_integrator.rb

@@ -29,7 +29,7 @@ module Pod
                 add_copy_resources_script_phase(native_target)
                 UserProjectIntegrator::TargetIntegrator.create_or_update_user_script_phases(script_phases_for_specs(test_specs), native_target)
               end
-              specs = target.specs.reject(&:test_specification?)
+              specs = target.non_test_specs
               UserProjectIntegrator::TargetIntegrator.create_or_update_user_script_phases(script_phases_for_specs(specs), target.native_target)
             end
           end

lib/cocoapods/project.rb

@@ -336,28 +336,26 @@ module Pod
       relative_base = base_path.nil? ? group.real_path : base_path.realdirpath
       relative_pathname = absolute_pathname.relative_path_from(relative_base)
       relative_dir = relative_pathname.dirname
-      lproj_regex = /\.lproj/i
 
       # Add subgroups for directories, but treat .lproj as a file
       if reflect_file_system_structure
         path = relative_base
         relative_dir.each_filename do |name|
-          break if name.to_s =~ lproj_regex
+          break if name.to_s.downcase.include? '.lproj'
           next if name == '.'
           # Make sure groups have the correct absolute path set, as intermittent
           # directories may not be included in the group structure
           path += name
-          group = group[name] || group.new_group(name, path)
+          group = group.children.find { |c| c.display_name == name } || group.new_group(name, path)
         end
       end
 
       # Turn files inside .lproj directories into a variant group
-      if relative_dir.basename.to_s =~ lproj_regex
+      if relative_dir.basename.to_s.downcase.include? '.lproj'
         group_name = variant_group_name(absolute_pathname)
         lproj_parent_dir = absolute_pathname.dirname.dirname
-        group = @variant_groups_by_path_and_name[[lproj_parent_dir, group_name]] ||
-          group.new_variant_group(group_name, lproj_parent_dir)
-        @variant_groups_by_path_and_name[[lproj_parent_dir, group_name]] ||= group
+        group = @variant_groups_by_path_and_name[[lproj_parent_dir, group_name]] ||=
+                  group.new_variant_group(group_name, lproj_parent_dir)
       end
 
       group

lib/cocoapods/resolver.rb

@@ -127,14 +127,19 @@ module Pod
       @resolver_specs_by_target ||= {}.tap do |resolver_specs_by_target|
         podfile.target_definition_list.each do |target|
           dependencies = {}
-          specs = target.dependencies.map(&:name).flat_map do |name|
+          specs = target.dependencies.flat_map do |dep|
+            name = dep.name
             node = @activated.vertex_named(name)
             (valid_dependencies_for_target_from_node(target, dependencies, node) << node).map { |s| [s, node.payload.test_specification?] }
           end
 
           resolver_specs_by_target[target] = specs.
             group_by(&:first).
-            map { |vertex, spec_test_only_tuples| ResolverSpecification.new(vertex.payload, spec_test_only_tuples.map { |tuple| tuple[1] }.all?, vertex.payload.respond_to?(:spec_source) && vertex.payload.spec_source) }.
+            map do |vertex, spec_test_only_tuples|
+              test_only = spec_test_only_tuples.all? { |tuple| tuple[1] }
+              spec_source = vertex.payload.respond_to?(:spec_source) && vertex.payload.spec_source
+              ResolverSpecification.new(vertex.payload, test_only, spec_source)
+            end.
             sort_by(&:name)
         end
       end
@@ -530,11 +535,15 @@ module Pod
       vertex = dependency_graph.vertex_named(dependency.name)
       predecessors = vertex.recursive_predecessors.select(&:root)
       predecessors << vertex if vertex.root?
-      platforms_to_satisfy = predecessors.flat_map(&:explicit_requirements).flat_map { |r| @platforms_by_dependency[r] }
+      platforms_to_satisfy = predecessors.flat_map(&:explicit_requirements).flat_map { |r| @platforms_by_dependency[r] }.uniq
+
+      available_platforms = spec.available_platforms
 
       platforms_to_satisfy.all? do |platform_to_satisfy|
-        spec.available_platforms.select { |spec_platform| spec_platform.name == platform_to_satisfy.name }.
-          all? { |spec_platform| platform_to_satisfy.supports?(spec_platform) }
+        available_platforms.all? do |spec_platform|
+          next true unless spec_platform.name == platform_to_satisfy.name
+          platform_to_satisfy.supports?(spec_platform)
+        end
       end
     end
 
@@ -549,14 +558,15 @@ module Pod
     def valid_dependencies_for_target_from_node(target, dependencies, node)
       dependencies[node.name] ||= begin
         validate_platform(node.payload, target)
-        dependency_nodes = node.outgoing_edges.select do |edge|
-          edge_is_valid_for_target?(edge, target)
-        end.map(&:destination)
-
-        dependency_nodes + dependency_nodes.flat_map do |item|
-          node_result = valid_dependencies_for_target_from_node(target, dependencies, item)
-          node_result
+        dependency_nodes = []
+        node.outgoing_edges.each do |edge|
+          next unless edge_is_valid_for_target?(edge, target)
+          dependency_nodes << edge.destination
         end
+
+        dependency_nodes.flat_map do |item|
+          valid_dependencies_for_target_from_node(target, dependencies, item)
+        end.concat dependency_nodes
       end
     end
 
@@ -566,9 +576,11 @@ module Pod
     # @return [Bool]
     #
     def edge_is_valid_for_target?(edge, target)
-      dependencies_for_target_platform =
-        edge.origin.payload.all_dependencies(target.platform).map(&:name)
-      dependencies_for_target_platform.include?(edge.requirement.name)
+      requirement_name = edge.requirement.name
+
+      edge.origin.payload.all_dependencies(target.platform).any? do |dep|
+        dep.name == requirement_name
+      end
     end
   end
 end

lib/cocoapods/sandbox/path_list.rb

 require 'active_support/multibyte/unicode'
+require 'find'
 
 module Pod
   class Sandbox
@@ -50,23 +51,23 @@ module Pod
         unless root.exist?
           raise Informative, "Attempt to read non existent folder `#{root}`."
         end
-        escaped_root = escape_path_for_glob(root)
-
-        absolute_paths = Pathname.glob(escaped_root + '**/*', File::FNM_DOTMATCH).lazy
-        dirs_and_files = absolute_paths.reject { |path| path.basename.to_s =~ /^\.\.?$/ }
-        dirs, files = dirs_and_files.partition { |path| File.directory?(path) }
 
+        dirs = []
+        files = []
         root_length = root.cleanpath.to_s.length + File::SEPARATOR.length
-        sorted_relative_paths_from_full_paths = lambda do |paths|
-          relative_paths = paths.lazy.map do |path|
-            path_string = path.to_s
-            path_string.slice(root_length, path_string.length - root_length)
-          end
-          relative_paths.sort_by(&:upcase)
+        Find.find(root.to_s) do |f|
+          directory = File.directory?(f)
+          f = f.slice(root_length, f.length - root_length)
+          next if f.nil?
+
+          (directory ? dirs : files) << f
         end
 
-        @dirs = sorted_relative_paths_from_full_paths.call(dirs)
-        @files = sorted_relative_paths_from_full_paths.call(files)
+        dirs.sort_by!(&:upcase)
+        files.sort_by!(&:upcase)
+
+        @dirs = dirs
+        @files = files
         @glob_cache = {}
       end
 

lib/cocoapods/target/pod_target.rb

@@ -49,7 +49,8 @@ module Pod
       raise "Can't initialize a PodTarget with only abstract TargetDefinitions" if target_definitions.all?(&:abstract?)
       raise "Can't initialize a PodTarget with an empty string scope suffix!" if scope_suffix == ''
       super()
-      @specs = specs
+      @specs = specs.dup.freeze
+      @test_specs, @non_test_specs = @specs.partition(&:test_specification?)
       @target_definitions = target_definitions
       @sandbox = sandbox
       @scope_suffix = scope_suffix
@@ -162,17 +163,18 @@ module Pod
     #         to this target.
     attr_reader :test_resource_bundle_targets
 
-    # @return [Bool] Whether or not this target should be build.
+    # @return [Bool] Whether or not this target should be built.
     #
-    # A target should not be build if it has no source files.
+    # A target should not be built if it has no source files.
     #
     def should_build?
       return @should_build if defined? @should_build
-      @should_build = begin
-        source_files = file_accessors.flat_map(&:source_files)
-        source_files -= file_accessors.flat_map(&:headers)
-        !source_files.empty? || contains_script_phases?
-      end
+
+      return @should_build = true if contains_script_phases?
+
+      source_files = file_accessors.flat_map(&:source_files)
+      source_files -= file_accessors.flat_map(&:headers)
+      @should_build = !source_files.empty?
     end
 
     # @return [Array<Specification::Consumer>] the specification consumers for
@@ -196,7 +198,7 @@ module Pod
     # @return [Array<Hash{Symbol=>String}>] An array of hashes where each hash represents a single script phase.
     #
     def script_phases
-      spec_consumers.map(&:script_phases).flatten
+      spec_consumers.flat_map(&:script_phases)
     end
 
     # @return [Boolean] Whether the target contains any script phases.
@@ -215,20 +217,16 @@ module Pod
     # @return [Boolean] Whether the target has any tests specifications.
     #
     def contains_test_specifications?
-      specs.any?(&:test_specification?)
+      !test_specs.empty?
     end
 
     # @return [Array<Specification>] All of the test specs within this target.
     #
-    def test_specs
-      specs.select(&:test_specification?)
-    end
+    attr_reader :test_specs
 
     # @return [Array<Specification>] All of the non test specs within this target.
     #
-    def non_test_specs
-      specs.reject(&:test_specification?)
-    end
+    attr_reader :non_test_specs
 
     # @return [Array<Symbol>] All of the test supported types within this target.
     #

spec/unit/generator/xcconfig/aggregate_xcconfig_spec.rb

@@ -438,7 +438,7 @@ module Pod
 
             it 'adds values from all subspecs' do
               @consumer_b.stubs(:user_target_xcconfig).returns('OTHER_CPLUSPLUSFLAGS' => '-std=c++1y')
-              consumer_c = mock(:user_target_xcconfig => { 'OTHER_CPLUSPLUSFLAGS' => '-stdlib=libc++' })
+              consumer_c = mock(:user_target_xcconfig => { 'OTHER_CPLUSPLUSFLAGS' => '-stdlib=libc++' }, :script_phases => [])
               @pod_targets[1].stubs(:spec_consumers).returns([@consumer_b, consumer_c])
               @xcconfig = @generator.generate
               @xcconfig.to_hash['OTHER_CPLUSPLUSFLAGS'].should == '-std=c++1y -stdlib=libc++'

spec/unit/installer/xcode/pods_project_generator/file_references_installer_spec.rb

@@ -188,9 +188,9 @@ module Pod
           describe 'Private Helpers' do
             describe '#file_accessors' do
               it 'returns the file accessors' do
-                pod_target_1 = PodTarget.new([stub('Spec')], [fixture_target_definition], config.sandbox)
+                pod_target_1 = PodTarget.new([stub('Spec', :test_specification? => false)], [fixture_target_definition], config.sandbox)
                 pod_target_1.file_accessors = [fixture_file_accessor('banana-lib/BananaLib.podspec')]
-                pod_target_2 = PodTarget.new([stub('Spec')], [fixture_target_definition], config.sandbox)
+                pod_target_2 = PodTarget.new([stub('Spec', :test_specification? => false)], [fixture_target_definition], config.sandbox)
                 pod_target_2.file_accessors = [fixture_file_accessor('banana-lib/BananaLib.podspec')]
                 installer = FileReferencesInstaller.new(config.sandbox, [pod_target_1, pod_target_2], @project)
                 roots = installer.send(:file_accessors).map { |fa| fa.path_list.root }
@@ -198,7 +198,7 @@ module Pod
               end
 
               it 'handles pods without file accessors' do
-                pod_target_1 = PodTarget.new([stub('Spec')], [fixture_target_definition], config.sandbox)
+                pod_target_1 = PodTarget.new([stub('Spec', :test_specification? => false)], [fixture_target_definition], config.sandbox)
                 pod_target_1.file_accessors = []
                 installer = FileReferencesInstaller.new(config.sandbox, [pod_target_1], @project)
                 installer.send(:file_accessors).should == []

spec/unit/installer_spec.rb

@@ -373,7 +373,7 @@ module Pod
           @analysis_result = Installer::Analyzer::AnalysisResult.new
           @analysis_result.specifications = []
           @analysis_result.sandbox_state = Installer::Analyzer::SpecsState.new
-          @spec = stub(:name => 'Spec')
+          @spec = stub(:name => 'Spec', :test_specification? => false)
           @spec.stubs(:root => @spec)
           @pod_targets = [PodTarget.new([@spec], [fixture_target_definition], config.sandbox)]
           @installer.stubs(:analysis_result).returns(@analysis_result)

spec/unit/sandbox/path_list_spec.rb

@@ -175,10 +175,12 @@ module Pod
       it 'orders paths case insensitively' do
         root = fixture('banana-unordered')
 
-        # Let Pathname.glob result be ordered case-sensitively
-        Pathname.stubs(:glob).returns([Pathname.new("#{root}/Classes/NSFetchRequest+Banana.h"),
-                                       Pathname.new("#{root}/Classes/NSFetchedResultsController+Banana.h")])
-        File.stubs(:directory?).returns(false)
+        # Let Find.find result be ordered case-sensitively
+        Find.stubs(:find).multiple_yields(
+          "#{root}/Classes",
+          "#{root}/Classes/NSFetchRequest+Banana.h",
+          "#{root}/Classes/NSFetchedResultsController+Banana.h",
+        )
 
         path_list = Sandbox::PathList.new(root)
         path_list.files.should == %w(Classes/NSFetchedResultsController+Banana.h Classes/NSFetchRequest+Banana.h)

spec/unit/target/aggregate_target_spec.rb

@@ -89,9 +89,9 @@ module Pod
         @spec = fixture_spec('banana-lib/BananaLib.podspec')
         @target_definition = Podfile::TargetDefinition.new('Pods', nil)
         @target_definition.abstract = false
+        @target_definition.set_platform(:ios, '10.0')
         @pod_target = PodTarget.new([@spec], [@target_definition], config.sandbox)
         @target = AggregateTarget.new(@target_definition, config.sandbox)
-        @target.stubs(:platform).returns(:ios)
         @target.pod_targets = [@pod_target]
       end