Speed up loading file statuses in Git commit dialog by batching events and skipping events for up-to-date files#9324
Conversation
mbien
added
git
mbien
left a comment
mbien
left a comment
There was a problem hiding this comment.
this looks good to me. Thanks a lot for the improvements.
left some comments inline but its nothing important
ide/git/src/org/netbeans/modules/git/ui/diff/MultiDiffPanelController.java
Outdated
Show resolved
Hide resolved
OndroMih
force-pushed
the
ondromih-git-commit-dlg-optimization-skip-no-update
branch
from
94bdb4a to
e48d9d6
Compare
|
@OndroMih for some reason I see an extra commit in this PR branch.
|
|
@mbien , I implemented the changes you requested, please review again. I added one more optimization - property change listeners run asynchronously - they both do a slow I/O operation for each file and running them asynchronously unblocks the main thread. This doesn't have impact on functionality because the final stage of updating the statuses was already asynchronous and it only updates the icons of files to reflect they status. Here's a sceenshot from profile before the change - the After these changes, when I forced update also on noupdate files, this decreased to 1 second: When I put back skipping update on noupdate files, the |
OndroMih
force-pushed
the
ondromih-git-commit-dlg-optimization-skip-no-update
branch
from
e48d9d6 to
2f76482
Compare
ide/versioning.core/src/org/netbeans/modules/versioning/core/VersioningAnnotationProvider.java
Outdated
Show resolved
Hide resolved
## Skip firing events for up-to-date Skip firing events for up-to-date files that are not yet in the cache, since UPTODATE is the default for managed files. This drastically reduces the time spent in the refreshStatusesBatch method on big repositories executed when Commit dialog opens. For example, on the Netbeans repository, from around 8 seconds to 20ms. ## Batch status change notifications Batch status change notifications to avoid per-file event overhead. Replace per-file PROP_FILE_STATUS_CHANGED firing in refreshStatusesBatch with a single PROP_FILES_STATUS_CHANGED batch event, eliminating 100k redundant propertyChange/schedule/SwingUtilities.invokeLater calls on first load. This improves performance a bit because it eliminates many method calls. However, in the end, the number of files changed is the same so the event handler still needs to process all of them. ## Move status updates to a background thread File status update requires I/O operation to refresh files metadata from FS. This is very slow when many files need to be updated. Moving them to a background thread offloads this slow operation from the main thread. Updates can run asynchronously without blocking the main thread that fires the status events, they just update UI hints, they have no impact no behavior. For the whole Netbeans repository, this shortens the time it takes to complete the firePropertyChange event from 6 seconds to 1 second.
OndroMih
force-pushed
the
ondromih-git-commit-dlg-optimization-skip-no-update
branch
from
2f76482 to
642c71b
Compare
|
the changes here do make sense to me, but I didn't really see a difference when i tested, see below.
how do you open the commit dialog (e.g right click on ?? -> commit) and how many files do you have in a modified state? I wasn't able to reproduce those kinds of performance gains but maybe the context of the commit dialog was different. What OS are we testing on? I noticed a few other things while testing but that would be for future PRs. Detailse.g: the method is called 11 times when a single file is modified and saved - and the actual refresh task runs 3 times. The original intention was likely to debounce events at that point, so that the task runs once. Its also unclear why it has to look at so many files when one file updates. (but this might be a side effect of the nbm ant project, haven't tested with maven) modified and saved |
|
@mbien , I always test by clicking on the "Commit All - Repository..." button in the toolbar. Or by selecting the root repository directory in the Favorites view and then "Team -> Commit..." in the menu. This invokes the Commit dialog for the whole repository, not only for selected subdirectory or a single selected file. This is the way how I always commit, to avoid skipping some files that I modified outside of the selected project that I forgot about. When doing commit per the whole Netbeans repository, the Commit dialog refreshes the whole file cache, going through all the files in the directory of the repository and its subfolders. When I do "Team -> Commit..." on when a single file is selected, or on a single project, e.g. on the Git Client Library project, the Commit dialog loads fast even without these changes. |
|
Fixing the whole logic of the |
thanks. its what I suspected but i wanted to be sure that we test the same thing ;) (the button isn't on the git toolbar by default). freshly cloned netbeans repo and modified a single java file. running
until the commit button enables. (JDK 26; linux)
yes I know that. I was just inspecting call sites to ensure that this change has no side effects. |
|
merging. thanks for the improvements! |
2 participants
Skip firing events for up-to-date
Skip firing events for up-to-date files that are not
yet in the cache, since UPTODATE is the default for managed files.
This drastically reduces the time spent in the refreshStatusesBatch method on big repositories executed when Commit dialog opens. For example, on the Netbeans repository, from around 8 seconds to 20ms.
Batch status change notifications
Batch status change notifications to avoid per-file event overhead.
Replace per-file PROP_FILE_STATUS_CHANGED firing in refreshStatusesBatch
with a single PROP_FILES_STATUS_CHANGED batch event, eliminating 100k
redundant propertyChange/schedule/SwingUtilities.invokeLater calls on
first load. This improves performance a bit because it eliminates many method calls.
However, in the end, the number of files changed is the same so the event handler
still needs to process all of them.
Move status updates to a background thread
File status update requires I/O operation to refresh files metadata from FS. This is very slow when many files need to be updated. Moving them to a background thread offloads this slow operation from the main thread.
Updates can run asynchrnously without blocking the main thread that fires the status events, they just update UI hints, they have no impact no behavior.
For the whole Netbeans repository, this shortens the time it takes to complete the firePropertyChange event from 6 seconds to 1 second.
Complements #9304, which speeds up the process even more, in a different area.
Click to collapse/expand PR instructions
PR approval and merge checklist:
If this PR targets the delivery branch: don't merge. (full wiki article)