Cache projectile-project-root for performance

[alexmurray]

Cache the result of (projectile-project-root) as a buffer local variable to
increase performance - this is implemented the same as was done
for (projectile-project-name) in PR 1906.

Should help resolve issue #1003

[fommil]

I'd like to give this a try because I've been hit by perf in the past.

[bbatsov]

You forgot the move the comment explaining this code here.

[alexmurray]

Ah thanks.

[bbatsov]

Why did you delete this?

[alexmurray]

I just moved it up earlier in the file so it is declared before we now first reference it.

[bbatsov]

No need for this. I can squash from the web ui.

[bbatsov]

I don't understand what exactly is this supposed to do. Seems like some unnecessary complication of the code.

[alexmurray]

To handle the case where there is no project root (and (projectile-project-root) returns nil) we can't just store nil as the cached value - as then we would end up reevaluating it every time and never using the cache. So instead we store the symbol 'none there instead - however we don't want to return 'none from the function since this invalidates the expected contract (return nil if not in a project) so we use cl-subst to replace any 'none value with nil.

[bbatsov]

Can't you simply use 'none as the default value for the buffer-local-variable?

[alexmurray]

But then we will never evaluate it - the code is:

(or (and (equal projectile-cached-buffer-file-name buffer-file-name)
          projectile-cached-project-root)
  ...)

So a non-nil value means the buffer-local cached value will be used - so it needs to be nil initially, then it gets evaluated once, and either set to a path or 'none and then it can be used as a cached value without reevaluation in the future.

[fommil]

Isn't it easier if nil just cache misses? So what, projectile is slightly slower on directories that are not source managed or have a manual .projectile :-/

There is an elisp package for caching btw, if you'd like to reuse it.

[alexmurray]

I disagree - I often use emacs on directories which are not part of a project and having EVERYTHING slow down (say when using spaceline-all-the-icons which calls (projectile-project-root) on every redisplay) is very annoying. Why go to the effort of caching in a project and not also add support for caching outside of a project?

Also it is more expensive to determine when we are outside of a project than inside a project since we can't short-circuit out of the checks - we have to exhaust every possible project test each time - so it is MORE of a win performance wise to cache in this case (ie when no project) than in the case of being in a project.

Which elisp package are you referring to?

[bbatsov]

Just explain in a comment what this code is doing for future reference and I'll merge it. No need for more 3rd party deps.

[alexmurray]

How about c473d17 ? Do you want me to squash these all into a single commit?

[bbatsov]

Thanks!

[aaronjensen]

Would it make sense to call this during projectile-invalidate-cache?

[Silex]

I wonder if it still makes sense to cache projectile-cached-project-name

[fommil]

that lib: https://github.com/skeeto/emacs-memoize

[fommil]

(in hindsight, I agree it makes sense to cache non-projects. I have worked in environments with network drives and ... short of having an extensive exclusion list for projectile... a cache for buffers in those folders would have surely helped)

[JeanMax]

Hey!

This merge broke (projectile-all-project-files) for me.
It now only returns the files of the current project... so (projectile-find-file-in-known-projects) acts as (projectile-find-file); 'projectile-known-projects is set correctly.

I could reproduce the behaviour with 'emacs -q' and 2 random projects, but it seems that it doesn't affect everybody.
linux / emacs 25.2.1