Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cache #27

Closed
dusans opened this issue Feb 10, 2014 · 29 comments
Closed

cache #27

dusans opened this issue Feb 10, 2014 · 29 comments
Assignees

Comments

@dusans
Copy link
Contributor

dusans commented Feb 10, 2014

Would it be possible to implement a cache for ctrlp-funky?

If the file didn't change since the last usage of ctrlp-funky there is no need to run the search again :)

This is noticeable when using ctrlp-funky on larger source files ( > 1000 lines) (python in my case)

@tacahiroy
Copy link
Owner

Yeah - indeed. Though I don't feel stress with > 10,000 lines objc code :)
But of course, it depends on machine capability, so I think that's useful for some people, uses old machine, edits massive source code etc.

If I have enough time this feature might be implemented, however, I don't know when.
Thanks for your idea!

@dusans
Copy link
Contributor Author

dusans commented Mar 13, 2014

I tested a bit and found out that the python regexp is slow.

Having only single line support makes the parsing a lot faster.

The python regexp was updated cuz of multiline support in issue: #12

function! ctrlp#funky#python#filters()
  let filters = [
        \ { 'pattern': '\v\C^\s*(def|class)\s+\w.+:',
        \   'formatter': ['\v\C^\s*', '', ''] },
  \ ]
  return filters
endfunction

@tacahiroy
Copy link
Owner

Or the current filter for python contains 2 items like this:

  let filters = [
        \ { 'pattern': '\v\C^\s*class\s+\w+\s*(\([^\)]+\))?:',
        \   'formatter': ['\v\C^\s*', '', ''] },
        \ { 'pattern': '\v\C^\s*def\s+\w+\s*(\_.*):',
        \   'formatter': ['\v\C^\s*', '', ''] }
  \ ]

I guess this is the one of the reasons the funky is slow, coz the funky parses a file 2 times.

@tacahiroy
Copy link
Owner

If you replace the filter function for python with below:

function! ctrlp#funky#python#filters()
  let filters = [
        \ { 'pattern': '\v\C^\s*(class\s+\w+\s*(\([^\)]+\))?|def\s+\w+\s*(\_.*)):',
        \   'formatter': ['\v\C^\s*', '', ''] }
  \ ]
  return filters
endfunction

Does it make the funky faster?

@dusans
Copy link
Contributor Author

dusans commented Mar 21, 2014

Will try when i get back to work. Thanks for the response.

@dusans
Copy link
Contributor Author

dusans commented Apr 8, 2014

Its still slow on large python files compared to single line version.

@dusans
Copy link
Contributor Author

dusans commented Apr 8, 2014

Its slow cuz it was greedy. Actually it selected the whole file :)

Having the second part non-greedy makes it fast and also support multiline functions.

function! ctrlp#funky#python#filters()
  let filters = [
        \ { 'pattern': '\v\C^\s*(class\s+\w+\s*(\([^\)]+\))?|def\s+\w+\s*(\_.{-})):',
        \   'formatter': ['\v\C^\s*', '', ''] }
  \ ]
  return filters
endfunction

@tacahiroy
Copy link
Owner

Finally, I've tested with a huge python file which has more than 10,000 lines.
Here is the results:

  • Current regex:
    • 15.352472
  • Improved regex (by @dusans):
    • 0.240578

As you say, non-greedy version is very very improved!! more than 70 times faster!!

Also, I've confirmed your improved regex capture both 1 line and multi-lines func defs properly.
Could you gimme a pull request pls. (to record your contribution).
Or I'll apply this change if you don't have time.

Thanks,

@dusans
Copy link
Contributor Author

dusans commented Apr 20, 2014

Done :)

@tacahiroy
Copy link
Owner

Cool - thanks :)

@dusans
Copy link
Contributor Author

dusans commented May 2, 2014

Just as a note. Its not that i have a slow PC (i have the latest i7).

The problem is that most of the files that i edit are on a remote PC. So it slow cuz of slow network/IO. Thats why a local cache would make a lot of difference in my environment. :)

Will try to look into how neocomplete does it and have that as the reference implementation.

@EDmitry
Copy link

EDmitry commented May 4, 2014

Yes, that would be a cool feature. Standard CtrlP ctags buffer definitely starts noticeably faster (immediately). I have relatively big Objective-C files that I edit every day.

@tacahiroy
Copy link
Owner

@dusans: i see that your situation.

ok - I think I'll implement this feature some time soon.

tacahiroy added a commit that referenced this issue May 19, 2014
Note: This implementation is rough, so refactoring required
@tacahiroy
Copy link
Owner

@dusans @EDmitry
As I committed, I've implemented this feature and pushed into a branch named add-cache-feature.
However, this is a first implementation, so possibly it contains some bugs.
In addition, I've tested on only my machine, OS X 10.9.2, so it doesn't work on Windows, probably.
Note that this feature uses sha256() function which came from Vim 7.4.

I'll improve and refactoring this implementation.

Pls lemme know if you notice any problems.

@dusans
Copy link
Contributor Author

dusans commented May 19, 2014

@tacahiroy thanks 👍
just tested. its much faster now (looks like the file is parsed only once)

but on my end "function sorting (on most used)" #15 doesn't work any more.

edit: now i see. This is only on the add-mru-func branch?

@tacahiroy
Copy link
Owner

Thanks for your feedback @dusans.

but on my end "function sorting (on most used)" #15 doesn't work any more.

indeed :/
I've just fixed the issue in this commit: 1826631

edit: now i see. This is only on the add-mru-func branch?

yes - this is WIP, so I haven't merged into master branch yet.

@dusans
Copy link
Contributor Author

dusans commented May 20, 2014

Tested. Looks fine to me.

As far as i see u check the sha256 of the whole file. I think u only need to check the last modified time-stamp.

@dusans
Copy link
Contributor Author

dusans commented May 20, 2014

i tested this and it works fine (for me) :)

diff --git a/autoload/ctrlp/funky.vim b/autoload/ctrlp/funky.vim
index f24bedf..735663b 100644
--- a/autoload/ctrlp/funky.vim
+++ b/autoload/ctrlp/funky.vim
@@ -88,8 +88,8 @@ function! s:cache.fname(bufnr, ...)
 endfunction

 function! s:cache.save(bufnr, defs)
-  let h = sha256(string(getbufline(bufname(a:bufnr), 1, '$')))
   let fname = self.fname(a:bufnr)
+  let h = getftime(fname)
   " save function defs
   let self.list[fname] = extend([h], a:defs)
   call writefile(self.list[fname], s:cache_dir . '/' . self.conv_name(fname))
@@ -121,8 +121,9 @@ function! s:cache.read(bufnr)
 endfunction

 function! s:cache.is_same_file(bufnr)
+  let fname = self.fname(a:bufnr)
   let prev_hash = self.hash(a:bufnr)
-  let cur_hash = sha256(string(getbufline(bufname(a:bufnr), 1, '$')))
+  let cur_hash = getftime(fname)
   return prev_hash == cur_hash
 endfunction

@tacahiroy
Copy link
Owner

indeed - I thought hash is more accurate, but timestamp is enough in almost of the case.
Thanks @dusans

@dusans
Copy link
Contributor Author

dusans commented May 20, 2014

@tacahiroy how about timestamp & file_size?

@tacahiroy
Copy link
Owner

cool - that idea is ok in case of 99% I guess.

@tacahiroy
Copy link
Owner

I've implemented the timestamp and file size way.
https://github.com/tacahiroy/ctrlp-funky/tree/add-cache-feature

The remaining tasks are

  • Test on Windows OS
  • Consider environments that path separator is backslash (and no shellslash env)

@dusans
Copy link
Contributor Author

dusans commented Jun 3, 2014

Great. Will test it on Windows 7 64-bit tomorrow 👍

@tacahiroy
Copy link
Owner

cool - but please gimme 1 day more, coz the code doesn't consider Windows environments at all.

@tacahiroy
Copy link
Owner

I've implemented Windows support for the cache feature.
I don't have any Windows machine, so I haven't tested on Windows. Probably, it should be working though.
c7ce5f3

FYI: You need to set g:ctrlp_funky_use_cache to 1 before you use the cache feature.

let g:ctrlp_funky_use_cache = 1

@dusans
Copy link
Contributor Author

dusans commented Jun 10, 2014

Tested on Windows. Works.

What is being saved to the .cache dir? Because there is only file with the name Z that is empty.
x:\Users\someuser.cache\ctrlp-funky\Z

@tacahiroy
Copy link
Owner

cool - basically, the funky saves cache files into the cache directory.
And a file name is based on a current file path like this:
%Users%tacahiroy%Projects%sample%Rakefile
Actually, I didn't consider non file buffer, so I guess you were editing unnamed buffer or something?

I need to implement the case for unnamed buffer.

@tacahiroy
Copy link
Owner

ah - no. unnamed/nofile buffers should be ignored, coz it doesn't have timestamp and file size .Also it's not a file ofc.

tacahiroy added a commit that referenced this issue Jun 24, 2014
tacahiroy added a commit that referenced this issue Jun 29, 2014
@tacahiroy
Copy link
Owner

I've just merged the branch into master: 373ff9a

Thanks guys

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants