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

Preserve trailing slash in maktaba#path#Split #199

Merged
merged 4 commits into from
Apr 29, 2017
Merged

Conversation

dbarnett
Copy link
Contributor

@dbarnett dbarnett commented Apr 29, 2017

Also improves maktaba#path#MakeRelative to preserve trailing slash and adds a maktaba#path#StripTrailingSlash().

A little profiling since I was concerned about hotspot performance regressions…
Profile summary before this change:

FUNCTIONS SORTED ON SELF TIME
count  total (s)   self (s)  function
  874   0.070503   0.042762  maktaba#path#Join()
 3296              0.042045  maktaba#path#AsDir()
   42   0.040494   0.025003  maktaba#rtp#LeafDirs()
  658   0.021195   0.014652  maktaba#setting#ParseHandle()
   41   0.054411   0.012718  maktaba#rtp#Join()
  559   0.018049   0.012574  maktaba#value#Focus()
  610   0.056664   0.012383  maktaba#plugin#Flag()
 3144              0.011213  maktaba#ensure#TypeMatches()
 2517              0.009126  maktaba#path#RootComponent()
  132   0.124043   0.007136  maktaba#plugin#GetOrInstall()
 2006   0.013524   0.006436  maktaba#ensure#IsList()
  147   0.009870   0.005441  maktaba#string#Strip()
  148   0.072684   0.005021  maktaba#function#Call()
   92   0.057603   0.004935  maktaba#plugin#Enter()
  171   0.005266   0.004645  maktaba#path#Split()
  610   0.023335   0.004571  maktaba#setting#Handle()
  175   0.005383   0.004011  maktaba#flags#Create()
  159   0.004876   0.003906  maktaba#rtp#Split()
   77   0.017688   0.003549  maktaba#plugin#AddonInfo()
   80   0.067771   0.003027  maktaba#plugin#Source()

Profile summary after this change:

FUNCTIONS SORTED ON SELF TIME
count  total (s)   self (s)  function
  874   0.070339   0.042532  maktaba#path#Join()
 2867              0.030666  maktaba#path#StripTrailingSlash()
   42   0.043343   0.026018  maktaba#rtp#LeafDirs()
  658   0.021416   0.014772  maktaba#setting#ParseHandle()
  610   0.057293   0.012579  maktaba#plugin#Flag()
  559   0.018096   0.012517  maktaba#value#Focus()
 3144              0.011521  maktaba#ensure#TypeMatches()
   41   0.041070   0.011328  maktaba#rtp#Join()
  808              0.010794  maktaba#path#AsDir()
 2517              0.009276  maktaba#path#RootComponent()
  132   0.125767   0.007175  maktaba#plugin#GetOrInstall()
 2006   0.013761   0.006481  maktaba#ensure#IsList()
  147   0.009712   0.005320  maktaba#string#Strip()
  148   0.080790   0.005243  maktaba#function#Call()
   92   0.058992   0.005129  maktaba#plugin#Enter()
  171   0.005646   0.005018  maktaba#path#Split()
  610   0.023532   0.004599  maktaba#setting#Handle()
  175   0.005448   0.004046  maktaba#flags#Create()
  159   0.004840   0.003883  maktaba#rtp#Split()
   77   0.017444   0.003549  maktaba#plugin#AddonInfo()

Actually seems to save a little overhead by replacing the s:StripTrailingSlash hack in rtp.vim with a simpler substitution (instead of using #path#AsDir and slicing). Certainly not a significant performance regression, at any rate.

Fixes #137 and #175.

Copy link
Member

@malcolmr malcolmr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

" <
" will echo
" - `['relative', 'path']`
" - `['/absolute/path']`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this be ['/absolute', 'path']?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch. Fixed.

@dbarnett
Copy link
Contributor Author

FYI, I found a hacky one-off s:StripTrailingSlash I could remove in favor of the real implementation, and then I did a little profiling because I was concerned about hotspot performance regressions (see updated PR description). Everything looks good there.

@dbarnett dbarnett merged commit 9dd8adb into master Apr 29, 2017
@dbarnett dbarnett deleted the split_trailing branch April 29, 2017 14:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants