Cache version result for each pod and segments #385
Conversation
dnkoutso
force-pushed
the
source_version_performance
branch
2 times, most recently
from
8bfd34e
to
db12586
Compare
lib/cocoapods-core/source.rb
Outdated
| @@ -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| | |||
There was a problem hiding this comment.
Can we use ||= here instead of checking the key and then setting it?
lib/cocoapods-core/source.rb
Outdated
| @@ -24,6 +24,7 @@ class Source | |||
| # | |||
| def initialize(repo) | |||
| @repo = Pathname(repo).expand_path | |||
| @name_to_versions_cache = {} | |||
There was a problem hiding this comment.
versions_by_name?
Also, can you see any place where this would need to be invalidated?
There was a problem hiding this comment.
@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.
There was a problem hiding this comment.
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
force-pushed
the
source_version_performance
branch
from
db12586
to
0efd5e3
Compare
|
Updated. |
dnkoutso
force-pushed
the
source_version_performance
branch
from
0efd5e3
to
5dc5247
Compare
|
Added spec. |
dnkoutso
force-pushed
the
source_version_performance
branch
from
5dc5247
to
de2feb7
Compare
|
Added invalidate cache option when calling |
CHANGELOG.md
Outdated
| @@ -15,6 +15,10 @@ | |||
|
|
|||
| ##### Enhancements | |||
|
|
|||
| * Cache version result for each pod and segments | |||
lib/cocoapods-core/source.rb
Outdated
| @@ -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 | |||
There was a problem hiding this comment.
Since this is a cache, I don't think we need to expose it publicly
There was a problem hiding this comment.
Yes, I thought about that too...removing
dnkoutso
force-pushed
the
source_version_performance
branch
from
de2feb7
to
1c687f3
Compare
|
All fixed. |
dnkoutso
force-pushed
the
source_version_performance
branch
from
1c687f3
to
282a7d1
Compare
dnkoutso
force-pushed
the
source_version_performance
branch
from
282a7d1
to
5cc9278
Compare
|
Rebased |
|
👍 nice! |
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.
There is another PR I will do in the main project.