Cache version result for each pod and segments #385

dnkoutso · 2017-06-17T05:26:22Z

The bigger our internal repo gets the more version comparisons occur. I profiled and saw a 17-20% increase by caching those results which seem safe to cache.

Here's a medium size project 17% improvement.

Before:
real	0m10.410s
user	0m9.156s
sys	0m0.772s

After:
real	0m8.642s
user	0m7.322s
sys	0m0.866s

There is another PR I will do in the main project.

segiddins · 2017-06-17T13:43:56Z

lib/cocoapods-core/source.rb

@@ -160,7 +161,8 @@ def versions(name)
      raise ArgumentError, 'No name' unless name
      pod_dir = pod_path(name)
      return unless pod_dir.exist?
-      pod_dir.children.map do |v|
+      return @name_to_versions_cache[name] if @name_to_versions_cache.key?(name)
+      @name_to_versions_cache[name] = pod_dir.children.map do |v|


Can we use ||= here instead of checking the key and then setting it?

segiddins · 2017-06-17T13:44:45Z

lib/cocoapods-core/source.rb

@@ -24,6 +24,7 @@ class Source
    #
    def initialize(repo)
      @repo = Pathname(repo).expand_path
+      @name_to_versions_cache = {}


versions_by_name?
Also, can you see any place where this would need to be invalidated?

@segiddins from a look, no I do not see it. The only case I can imagine we might need to invalidate the cache is when we perform update of the repo in source.rb.

However, updating occurs at the beginning before querying for specs.

Having said that, it might be correct for us to add code that invalidates when the repo is updated. Just because cocoapods installer calls things in the right order doesn't mean it should be designed as such.

dnkoutso · 2017-06-17T16:38:52Z

Updated.

dnkoutso · 2017-06-17T17:17:54Z

Added spec.

dnkoutso · 2017-06-18T03:11:52Z

Added invalidate cache option when calling update on a repo to be sure. Also added a test.

segiddins · 2017-06-18T03:15:09Z

CHANGELOG.md

@@ -15,6 +15,10 @@

 ##### Enhancements

+* Cache version result for each pod and segments  


Wrong section?

segiddins · 2017-06-18T03:15:54Z

lib/cocoapods-core/source.rb

@@ -20,10 +20,15 @@ class Source
    #
    attr_reader :metadata

+    # @return [Hash{String=>Array<Version>}] The cache for each pod name to its versions.
+    #
+    attr_reader :versions_by_name


Since this is a cache, I don't think we need to expose it publicly

Yes, I thought about that too...removing

dnkoutso · 2017-06-18T04:01:04Z

All fixed.

dnkoutso · 2017-06-20T16:09:04Z

Rebased

segiddins · 2017-06-20T16:29:30Z

👍 nice!

dnkoutso force-pushed the source_version_performance branch 2 times, most recently from 8bfd34e to db12586 Compare June 17, 2017 05:34

segiddins reviewed Jun 17, 2017

View reviewed changes

dnkoutso force-pushed the source_version_performance branch from db12586 to 0efd5e3 Compare June 17, 2017 16:38

dnkoutso force-pushed the source_version_performance branch from 0efd5e3 to 5dc5247 Compare June 17, 2017 17:17

dnkoutso force-pushed the source_version_performance branch from 5dc5247 to de2feb7 Compare June 18, 2017 03:11

segiddins reviewed Jun 18, 2017

View reviewed changes

CHANGELOG.md Outdated

@@ -15,6 +15,10 @@

##### Enhancements

* Cache version result for each pod and segments

Copy link

Member

segiddins Jun 18, 2017

Choose a reason for hiding this comment

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

Wrong section?

segiddins reviewed Jun 18, 2017

View reviewed changes

dnkoutso force-pushed the source_version_performance branch from de2feb7 to 1c687f3 Compare June 18, 2017 04:00

dnkoutso force-pushed the source_version_performance branch from 1c687f3 to 282a7d1 Compare June 18, 2017 15:14

Cache version result for each pod and segments

5cc9278

dnkoutso force-pushed the source_version_performance branch from 282a7d1 to 5cc9278 Compare June 20, 2017 16:08

dnkoutso merged commit f085263 into CocoaPods:master Jun 20, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Cache version result for each pod and segments #385

Cache version result for each pod and segments #385

dnkoutso commented Jun 17, 2017 •

edited

Loading

segiddins Jun 17, 2017

segiddins Jun 17, 2017

dnkoutso Jun 17, 2017

dnkoutso Jun 17, 2017

dnkoutso Jun 17, 2017

dnkoutso commented Jun 17, 2017

dnkoutso commented Jun 17, 2017

dnkoutso commented Jun 18, 2017

segiddins Jun 18, 2017

segiddins Jun 18, 2017

dnkoutso Jun 18, 2017

dnkoutso commented Jun 18, 2017

dnkoutso commented Jun 20, 2017

segiddins commented Jun 20, 2017

		@@ -15,6 +15,10 @@

		##### Enhancements

		* Cache version result for each pod and segments

Cache version result for each pod and segments #385

Cache version result for each pod and segments #385

Conversation

dnkoutso commented Jun 17, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dnkoutso commented Jun 17, 2017

dnkoutso commented Jun 17, 2017

dnkoutso commented Jun 18, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dnkoutso commented Jun 18, 2017

dnkoutso commented Jun 20, 2017

segiddins commented Jun 20, 2017

dnkoutso commented Jun 17, 2017 •

edited

Loading