cache location of session file

[rleon]

Function session_file() returns the name of session file.
This function is called for every change in VIM layout.

Caching of return variable of this function gives x100 performance
improvement.

Before change:
FUNCTIONS SORTED ON TOTAL TIME
count total (s) self (s) function
6 3.369383 0.015592 g:GitSessionUpdate()
6 3.353791 0.000861 35_session_file()
6 3.268404 0.001057 35_session_dir()
12 3.263580 0.004735 35_in_git_repo()

After this change:
FUNCTIONS SORTED ON TOTAL TIME
count total (s) self (s) function
11 0.026195 g:GitSessionUpdate()
23 0.025646 38_Highlight_Matching_Pair()
11 0.000956 nerdtree#checkForBrowse()

VIM profiling (http://stackoverflow.com/a/12216578):
:profile start profile.log
:profile func *
:profile file *
" At this point do slow actions
:profile pause
:noautocmd qall!

Signed-off-by: Leon Romanovsky leon@leon.nu

[wting]

Yeah, we're hitting the disk multiple times on when switching buffers. From your profiling it looks like +550 ms per buffer change, which is kind of excessive (maybe it's time to move to neovim for async). Was this tested on a spinning disk / network mount? Thanks for doing the leg work!

One concern is if a user switches branches but keeps vim open, they'll unknowingly continue saving to the previous session. I'm not sure if this is desired behavior or not. Currently the user doesn't have to think about invalidating caches when switching git branches.

If you still think this is worth pursuing, I have a few suggestions to make:

• make this behavior controlled by an option and disabled by default
• instead of using the cached session file everywhere, only cache the primary offender (GitSessionUpdate)
• invalidate the cache on GitSessionSave since that's a manually triggered action

[rleon]

Thank you for taking time to review and provide such extensive feedback.

My testing environment is local SSD which is almost full (no space for over-provisioning) and the reason to such a large delays is related to the size of git repository (linux kernel).

I suggest to rewrite the session_file() function to return cached version if global variable is set. In case this variable is not set, the default behaviour will be performed.

Regarding the session change per branch, it was the easiest approach to fix performance issues and it fits perfectly my flow. I am open for suggestions how to get information on branch change in order to update session file location.

[wting]

Rewriting session_file() to use a cache is probably an easier to understand solution, and basically always invalidate the cache in GitSessionSave.

The more I think about it, the more I prefer this behavior. I've accidentally overwritten the wrong session because I leave vim open. I just want to put this behind a feature toggle so we can test this for a bit before making it the default behavior if everything looks good.

[rleon]

I agree with you, it is more convenient to user to save session once and
use it for all branches.

The main concern is how to handle files which are available and open in one
branch and not available in other.

In any cases, i'll update the pull request in day or two.

Thanks
On Jul 23, 2015 9:33 PM, "William Ting" notifications@github.com wrote:

Rewriting session_file() to use a cache is probably an easier to
understand solution, and basically always invalidate the cache in
GitSessionSave.

The more I think about it, the more I prefer this behavior. I've
accidentally overwritten the wrong session because I leave vim open. I just
want to put this behind a feature toggle so we can test this for a bit
before making it the default behavior if everything looks good.

—
Reply to this email directly or view it on GitHub
#10 (comment).

[rleon]

I pushed new version of this feature.
Please review it.

Thanks.

[rleon]

set cached to be disabled

[wting]

Thanks for the patch and cleaning it up!

Just a heads up, I've renamed the caching option name to gitsessions_use_cache.

[wting]

I've updated docs here. Lemme know if it sounds weird or can be improved.

[rleon]

Thank you.
It looks fine for me.
I forgot to overlap g:cache_session_file variable.
Can you please apply this change on top of my commit.
diff --git a/plugin/gitsessions.vim b/plugin/gitsessions.vim
index aa8a483..0f7e109 100644
--- a/plugin/gitsessions.vim
+++ b/plugin/gitsessions.vim
@@ -36,7 +36,9 @@ endif
" Cons: switch between git branches will be missed from GitSessionUpdate()
" You are advised to save it manually by calling to GitSessionSave()
" Default - cache disabled
-let g:cache_session_file = 0
+if !exists('g:cache_session_file')

• let g:cache_session_file = 0
  +endif

" HELPER FUNCTIONS

[wting]

Yup, already added that in 9e444bc.

[rleon]

Thanks

[wting]

FYI this is the new default behavior as of d4e8c65.

[rleon]

Thank you,
I appreciate it.