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

perf: cache .git/index for better performance #787

Merged
merged 4 commits into from
Jul 31, 2019

Conversation

billiegoose
Copy link
Member

I'm improving performance

This speeds up running statusMatrix on large repos like https://github.com/ianstormtaylor/slate from ~80 seconds to ~1 second. (These times measured on a proprietary Electron app. How big the perf hit is depends highly on the latency of the filesystem... on Electron the latency is very bad because it is proxied to the worker over IPC.)

Possible Dangers

It does caching based on the compound key [fs, filepathOfIndexFile]. It uses lstat to determine whether the index file has been modified. 99% of the time this should be enough! However... if you are "resetting" a filesystem mock behind the scenes (like my BrowserFS tests were), that might not be enough. In such cases that the index file returns the same stat information but it actually has changed different, then your tests may too fail. (This caching may also be susceptible to "racy-git"-like race conditions, since it depends on the last-modified timestamp.)

The solution is to make sure whenever you are "resetting" the filesystem to set the filesystem to a new object, i.e.

plugins.set('fs', {...fs})

so that the old cached index file will not be used.

@isomorphic-git-bot
Copy link

Thank you wmhilton! I auto-formatted the code using prettier-standard. 🤖

@karma-pr-reporter
Copy link

Test Results for 563b3e7:

Browser Passed Skipped Failed Time Disconnected Error
HeadlessChrome 0.0.0 (Linux 0.0.0) 265 1 0 1 min 19.605 secs false undefined
Mobile Safari 11.0.0 (iOS 11.2.0) 81 0 0 2 mins 5.011 secs false undefined
Mobile Safari 11.0.0 (iOS 11.2.0) 81 0 0 2 mins 5.043 secs false undefined
Firefox 67.0.0 (Ubuntu 0.0.0) 265 1 0 2 mins 6.456 secs false undefined
Edge 17.17134.0 (Windows 10 0.0.0) 81 0 0 2 mins 23.464 secs false undefined
Edge 17.17134.0 (Windows 10 0.0.0) 81 0 0 2 mins 23.479 secs false undefined
Safari 11.1.2 (Mac OS X 10.13.6) 265 1 0 3 mins 40.967 secs false undefined
Chrome Mobile 67.0.3396 (Android 7.1.1) 264 1 1 4 mins 6.131 secs false undefined

@karma-pr-reporter
Copy link

Test Results for 563b3e7:

Browser Passed Skipped Failed Time Disconnected Error
Mobile Safari 11.0.0 (iOS 11.2.0) 81 0 0 2 mins 13.696 secs false undefined
Mobile Safari 11.0.0 (iOS 11.2.0) 81 0 0 2 mins 13.718 secs false undefined
Edge 17.17134.0 (Windows 10 0.0.0) 81 0 0 2 mins 16.07 secs false undefined
Edge 17.17134.0 (Windows 10 0.0.0) 81 0 0 2 mins 16.078 secs false undefined
Chrome Mobile 67.0.3396 (Android 7.1.1) 264 1 1 3 mins 43.29 secs false undefined

@karma-pr-reporter
Copy link

Test Results for e365122:

Browser Passed Skipped Failed Time Disconnected Error
HeadlessChrome 0.0.0 (Linux 0.0.0) 265 1 0 1 min 16.391 secs false undefined
Firefox 67.0.0 (Ubuntu 0.0.0) 265 1 0 2 mins 0.016 secs false undefined
Edge 17.17134.0 (Windows 10 0.0.0) 265 1 0 5 mins 17.983 secs false undefined

@karma-pr-reporter
Copy link

Test Results for e365122:

Browser Passed Skipped Failed Time Disconnected Error
Chrome Mobile 67.0.3396 (Android 7.1.1) 265 1 0 3 mins 17.253 secs false undefined
Safari 11.1.2 (Mac OS X 10.13.6) 265 1 0 3 mins 51.82 secs false undefined
Mobile Safari 11.0.0 (iOS 11.2.0) 265 1 0 4 mins 29.894 secs false undefined

@karma-pr-reporter
Copy link

Test Results for a3d50e7:

Browser Passed Skipped Failed Time Disconnected Error
HeadlessChrome 0.0.0 (Linux 0.0.0) 265 1 0 1 min 19.512 secs false undefined
Firefox 67.0.0 (Ubuntu 0.0.0) 265 1 0 2 mins 11.537 secs false undefined
Safari 11.1.2 (Mac OS X 10.13.6) 265 1 0 2 mins 45.956 secs false undefined
Chrome Mobile 67.0.3396 (Android 7.1.1) 265 1 0 3 mins 51.553 secs false undefined
Mobile Safari 11.0.0 (iOS 11.2.0) 265 1 0 4 mins 21.484 secs false undefined
Edge 17.17134.0 (Windows 10 0.0.0) 265 1 0 5 mins 30.32 secs false undefined

@billiegoose billiegoose merged commit b2e242d into master Jul 31, 2019
@billiegoose billiegoose deleted the perf/cache-GitIndex branch July 31, 2019 03:02
@isomorphic-git-bot
Copy link
Member

🎉 This PR is included in version 0.58.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

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