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

Constant repository polling for changes #4069

Closed
RussKie opened this issue Oct 19, 2017 · 12 comments
Closed

Constant repository polling for changes #4069

RussKie opened this issue Oct 19, 2017 · 12 comments

Comments

@RussKie
Copy link
Member

RussKie commented Oct 19, 2017

Do you want to request a feature or report a bug?
bug, related to some extent to #3836 and #3774.

What is the current behavior?
When "Show number of changed files on commit button" is enabled GE is constantly invoking git status --porcelain -z --untracked-files which has a significant performance impact for large repos. Git processes are constantly kicked off and those typically spike CPU usage by 20-30%.

image

If "Show number of changed files on commit button" is disabled the problem seems to go away, however it is a workaround not a solution to the problem.

What is the expected behavior?
Polling for changes should be configurable and perhaps have some sort of a throttling mechanism

Environment you encounter the issue:

  • GitExtensions version: 2.50
  • GIT version: 2.14
  • OS version: W8.1

Did this work in previous version of GitExtensions (which)?
I have seem it in 2.4x versions

@gerhardol
Copy link
Member

gerhardol commented Oct 22, 2017

Do you get the same with FileSystemWatcher enabled?

A source comment in ToolStripGitStatus mentions that the command should run every 5 min or at changes. I cannot see any obvious flaws.

I do not see the constant polling, with or without FileSystemWatcher. But repos are quite small.

Edit: One thing in common seem to be that commands seem duplicated, often appears in identical pairs.

@RussKie
Copy link
Member Author

RussKie commented Oct 22, 2017

I hadn't investigated the source in details, my current understanding that "Show number of changed files on commit button" enables the FileSystemWatcher.

Our repo is large (5+ years, 90 projects in a solution). I think whenever the source is getting compiled and processed by VS/grunt, filesystem i/o set the watcher off like crazy...

I've noticed the log duplication too. Not too concerned with that atm.

@gerhardol
Copy link
Member

While I have not seen so high CPU usage, the handling with many submodules is a little slow (this is still a good feature in GitExtensions).

The FileSystemWatcher option AppSettings.UseFastChecks seem to be an additional service that set an exclamation mark on the refresh button when the file system has changed. There does not seem to be much overhead here. The RevisionGrid is otherwise updated when GE has done operations that affect the index.

There is also a FileSystemWatcher in ToolStripGitStatus Settings.ShowGitStatusInBrowseToolbar, controlled by "show number of changed files on commit button". The purpose is to update the counter. This will run "git status" at every file change or every fifth minute. There is a deferred delay of 500 ms when Git operations start, but if there are continuous changes to the file system, the check can run more or less continuous too. The "git status" check can take quite some time. The GE repo is not that big but with many submodules, it could be slow.

A discussion to improve the functionality:

  1. The update must be done much less often. How long can we wait to update the status after change? Is 5s OK? Must this be configurable?
    The changes below in ToolStripGitStatus,cs increases the time.
    @RussKie Can you try?

As a temporary workaround, the FileSystemWatcher can be used to give the exclamation mark when there are file changes.

  1. Change the default text and default option for FileSystemWatcher to true and add new to continuously update the count on the button. New default or hardcoded option to add the count whenever the revision grid is updated.
    If the update time is configurable, the time can be used to control if continuous update or only change when revision grid is updated.

  2. With Enhanced view of uncommitted changes in Browse Repository #4031 I would like to put the changed files in the artificial commit text. It would be nice to share the FileSystemWatcher here with the button (the exclamation check can run faster).
    This could also be used to improve the performance when opening the submodule dropdown, which is my need.

     private void ScheduleDeferredUpdate()
     {
         _nextUpdateTime = Environment.TickCount + UpdateDelay*10;
     }
    
     private void ScheduleImmediateUpdate()
     {
         _nextUpdateTime = Environment.TickCount + UpdateDelay*10;
     }
    

gerhardol added a commit to gerhardol/gitextensions that referenced this issue Oct 23, 2017
…use FileSystemWatcher) is activated

This change originates in gitextensions#4031 but can also be used as a workaround for limitations due to gitextensions#4069
gerhardol added a commit to gerhardol/gitextensions that referenced this issue Oct 23, 2017
…use FileSystemWatcher) is activated

This change originates in gitextensions#4031 but can also be used as a workaround for limitations due to gitextensions#4069
@gerhardol
Copy link
Member

gerhardol commented Nov 5, 2017

Update: Will probably not go into caching of "dynamic" commands, it will have too much impact on for instance testing.

I propose this handling is improved in a way that many can benefit from the changes. Discussed in other issues too

Brief proposal:

  • Centralize/cache all calls to Module.GetAllChangedFiles() and related (basically git status). The caller can set a max time for the data or it is refreshed (some override primarily for the refresh button too).
  • Callers can register callbacks for updated GetAllChangedFiles() with minimum update rate. The updates can be used by Commit count as well as the artificial count (Show count for artificial commits #4086), maybe even update the diff filelist. The refresh button can set the exclamation mark.
  • FileSystemWatcher will note the last change in the file system and only call git status when changes have been done to the file system
  • 'git status' will not run more often and call the registered callback function than a user configurable value allows (the command can run continuously if files are compiled for instance, there are constantly changes.

@gerhardol
Copy link
Member

I tried to debug why the same command seem to be logged two times. It does not seem to be called twice, it seems to be that the logger in GitCommandHelpers.cs StartProcess fires Exited twice, with the same information.

For the performance problem, I have added a delay until next command start, but it will not change the situation much if git status requires 1,5s.
The command is running on a threadpool, so a new threadpool is required to lower the priority.

So the "solution" is probably a setting "max time to update the Commit count at file system changes". Default set to 10s or so?

@RussKie
Copy link
Member Author

RussKie commented Dec 6, 2017

To be honest, I haven't had time to give it any thought...
How easy is it to fix the dup logs?

@gerhardol
Copy link
Member

How easy is it to fix the dup logs?

I only saw crude fixes. As I found no other reports with this problem (only slightly different), there should be a real explanation.

@RussKie
Copy link
Member Author

RussKie commented Dec 6, 2017 via email

@gerhardol
Copy link
Member

I see dup logs on all my systems....

So do I. I plan look at this again later.

As for the long execution - I think it is a problem with our work laptops.

The virus program most likely.
I am playing around with some dynamic setting, to wait at least twice as long time as the update took before starting a new one. Then sporadic updates are still quick but updates with virus slowing down will not eat all execution time. (Need to decrease the complexity a little...)

@RussKie
Copy link
Member Author

RussKie commented Dec 6, 2017

This one isn't particularly high on my todo list, there are few other items of much higher value to our users (mainly UX related).

gerhardol added a commit to gerhardol/gitextensions that referenced this issue Dec 9, 2017
…ent selection is lost

Improves/resolves gitextensions#4069 where file system is constantly updated and git-status is slow
@gerhardol
Copy link
Member

A proposal based on #4209, will submit PR when merged
Not perfect but still responds within one sec for the first change and five sec at constant updates should be resonable

gerhardol added a commit to gerhardol/gitextensions that referenced this issue Dec 31, 2017
Reported in gitextensions#4069
Based on changes in gitextensions#4209
GE can in some situations  use a lot of CPU and disk load, as it runs "git status --untracked-files" at changes in the filesystem
If a status command is not done before next update is detaected, the next start immediately after. "git status" can therefore run continuously.
(This is more visible with certain viruses like Symantec and McAfee.)

This PR limits the checks in two ways:
 * First run is started after 1000 ms + timer (average 250ms) instead of 500+timer, to collect more changes (probably not essential)
 * Next command is started minimum 5000ms after the previous command.
So normal one shot changes are detected about the same as now, but continuous changes are not updating so frequently.

A few other changes like that the "next run" time could be overwritten by a time in the future and that retrieved "git status" data were not used if more file system changes were detected.

If "git status" requires more than 5s to run, the behavior is not improved. The counter in the Commit button should not be used then. Configurable time could improve (like longest of 5s and twice the time to run git status), configurable time is not going to be used.

There are situations where the "ignored files detection" is insufficient. See gitextensions#4256 for a discussion.
gerhardol added a commit to gerhardol/gitextensions that referenced this issue Jan 2, 2018
Reported in gitextensions#4069
Based on changes in gitextensions#4209
GE can in some situations  use a lot of CPU and disk load, as it runs "git status --untracked-files" at changes in the filesystem
If a status command is not done before next update is detaected, the next start immediately after. "git status" can therefore run continuously.
(This is more visible with certain viruses like Symantec and McAfee.)

This PR limits the checks in two ways:
 * First run is started after 1000 ms + timer (average 250ms) instead of 500+timer, to collect more changes (probably not essential)
 * Next command is started minimum 5000ms after the previous command.
So normal one shot changes are detected about the same as now, but continuous changes are not updating so frequently.

A few other changes like that the "next run" time could be overwritten by a time in the future and that retrieved "git status" data were not used if more file system changes were detected.

If "git status" requires more than 5s to run, the behavior is not improved. The counter in the Commit button should not be used then. Configurable time could improve (like longest of 5s and twice the time to run git status), configurable time is not going to be used.

There are situations where the "ignored files detection" is insufficient. See gitextensions#4256 for a discussion.
gerhardol added a commit to gerhardol/gitextensions that referenced this issue Jan 7, 2018
Reported in gitextensions#4069
GE can in some situations  use a lot of CPU and disk load, as it runs "git status --untracked-files" at changes in the filesystem
If a status command is not done before next update is detected, the next start immediately after. "git status" can therefore run continuously.
(This is more visible with certain viruses like Symantec and McAfee.)

This PR limits the checks in two ways:
 * First run is started after 1000ms + timer (fires every 500ms so adds on average 250ms) instead of 500ms +timer, to collect more changes (probably not essential)
 * Next command is started minimum 3000ms after the previous command. If 3*time to run the status update is longer than 3000ms, the interval time is dynamically increased.
So normal one shot changes are detected about the same as now, but continuous changes are not updating so frequently, "git status" should not run continuously.

A few other changes like that the "next run" time could be overwritten by a time in the future and that retrieved "git status" data were not used if more file system changes were detected.

If "git status" requires more than 5s to run, the behavior is not improved. The counter in the Commit button should not be used then. Configurable time could improve (like longest of 5s and twice the time to run git status), configurable time is not going to be used.

There are situations where the "ignored files detection" is insufficient. See gitextensions#4256 for a discussion.
gerhardol added a commit to gerhardol/gitextensions that referenced this issue Jan 31, 2018
Reported in gitextensions#4069
GE can in some situations  use a lot of CPU and disk load, as it runs "git status --untracked-files" at changes in the filesystem
If a status command is not done before next update is detected, the next start immediately after. "git status" can therefore run continuously.
(This is more visible with certain viruses like Symantec and McAfee.)

This PR limits the checks in two ways:
 * First run is started after 1000ms + timer (fires every 500ms so adds on average 250ms) instead of 500ms +timer, to collect more changes (probably not essential)
 * Next command is started minimum 3000ms after the previous command. If 3*time to run the status update is longer than 3000ms, the interval time is dynamically increased.
So normal one shot changes are detected about the same as now, but continuous changes are not updating so frequently, "git status" should not run continuously.

A few other changes like that the "next run" time could be overwritten by a time in the future and that retrieved "git status" data were not used if more file system changes were detected.

If "git status" requires more than 5s to run, the behavior is not improved. The counter in the Commit button should not be used then. Configurable time could improve (like longest of 5s and twice the time to run git status), configurable time is not going to be used.

There are situations where the "ignored files detection" is insufficient. See gitextensions#4256 for a discussion.
RussKie pushed a commit that referenced this issue Feb 1, 2018
* Limit how often GitStatus polls for changes

Reported in #4069
GE can in some situations  use a lot of CPU and disk load, as it runs "git status --untracked-files" at changes in the filesystem
If a status command is not done before next update is detected, the next start immediately after. "git status" can therefore run continuously.
(This is more visible with certain viruses like Symantec and McAfee.)

This PR limits the checks in two ways:
 * First run is started after 1000ms + timer (fires every 500ms so adds on average 250ms) instead of 500ms +timer, to collect more changes (probably not essential)
 * Next command is started minimum 3000ms after the previous command. If 3*time to run the status update is longer than 3000ms, the interval time is dynamically increased.
So normal one shot changes are detected about the same as now, but continuous changes are not updating so frequently, "git status" should not run continuously.

A few other changes like that the "next run" time could be overwritten by a time in the future and that retrieved "git status" data were not used if more file system changes were detected.

If "git status" requires more than 5s to run, the behavior is not improved. The counter in the Commit button should not be used then. Configurable time could improve (like longest of 5s and twice the time to run git status), configurable time is not going to be used.

There are situations where the "ignored files detection" is insufficient. See #4256 for a discussion.
@RussKie
Copy link
Member Author

RussKie commented Nov 14, 2018

Closing due to age.
v3 doesn't appear as chatty as old versions, assuming @gerhardol's magic worked.

@RussKie RussKie closed this as completed Nov 14, 2018
@RussKie RussKie modified the milestones: 3.00, 3.00-beta1 Nov 14, 2018
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

2 participants