add option use-git-cache which makes single git log ... call
#87
Conversation
This diff does two things: 1. aligns path_cache behavior in determinator.rb so that both formatted and raw calls to last_modified_time are read from the path_cache 2. adds option `use-git-cache` which improves render performance by reading the entire git log once (instead of 1:1 for each file)
`item.path` returns the absolute path to a file, whereas other instantiations of `Determinator`, like in `tag.rb`, use a relative path. by using a relative path in both places we increase the likelihood of cache hits.
| @@ -3,49 +3,47 @@ | |||
| module Jekyll | |||
| module LastModifiedAt | |||
| class Determinator | |||
| attr_reader :site_source, :page_path | |||
| @repo_cache = {} | |||
| @path_cache = {} | |||
There was a problem hiding this comment.
This is actually a last modified time cache keyed on paths. @last_mod_cache perhaps?
| raise Errno::ENOENT, "#{absolute_path_to_article} does not exist!" unless File.exist? absolute_path_to_article | ||
|
|
||
| Time.at(last_modified_at_unix.to_i) | ||
| self.class.path_cache[page_path] = Time.at(last_modified_at_unix.to_i) | ||
| self.class.path_cache[page_path] |
There was a problem hiding this comment.
ruby returns the last value by default, so this last line is redundant, no?
| @@ -8,6 +8,7 @@ class Git | |||
| def initialize(site_source) | |||
| @site_source = site_source | |||
| @is_git_repo = nil | |||
| @lcd_cache = {} | |||
There was a problem hiding this comment.
What does "lcd" stand for here? As opposed to what other kind of cache?
There was a problem hiding this comment.
I reckon it stands for "last commit date", per the next hunk
| } | ||
| end | ||
|
|
||
| Jekyll::Hooks.register :site, :after_reset do |site| |
There was a problem hiding this comment.
I personally didn't find this hook very helpful: it triggers a cache clear every file change while I'm developing, slowing my incremental builds and with no benefit, as uncommitted changes don't show up in git log
|
👋 Unfortunately, my GitHub notifications for this repository was oddly turned off. I take my responsibility to accepting patches seriously; however, I am just one person, with a very busy life off of GitHub.com! I'm going to close this PR because I like to keep my TODO list scoped and reasonable. This does not mean I will not accept this patch! Rather, it's a way of me asking you if you're still using this gem, and whether you want this PR merged. Just comment back and I'll prioritize it. I assume silence means I can keep it closed. Thanks! this was an automated copy-paste |
|
I'm still using this PR |
|
Sorry, should have mentioned: I built on this PR in #91 - If you just merge 91, it should come with this functionality... up to you how to handle it 😊 |
This diff does two things:
aligns path_cache behavior in determinator.rb so that both formatted and raw
calls to last_modified_time are read from the path_cache
adds option
use-git-cachewhich improves render performance by reading theentire git log once (instead of 1:1 for each file)
More information available in #85
I did not test extensively but did verify that
bundle exec rake specpassed.bundle exec rake rubocopreported an issue withStyle/OptionalBooleanParameterwhich I did not think worth changing existing code to accommodate.