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
| for (ChangedEvent event : events) { | ||
| fireFileStatusChanged(event); | ||
| if (!events.isEmpty()) { | ||
| listenerSupport.firePropertyChange(PROP_FILES_STATUS_CHANGED, null, events); |
There was a problem hiding this comment.
good catch. aggregating events can certainly reduce overhead under load. Reminds me a little on #8955 where I saw 1.5mil event handler invocations when a large project group opened.
There was a problem hiding this comment.
Actually, the overhead reduction is almost negligible. I was tempted to remove this but then I found out one more way to optimize this, by running the property change listeners asynchronously in background thread.
Each single file status update is an I/O operation and thus slow. Running the updates in background moves this off the main thread. Running everything in batch allows offloading everything to a single background thread instead of updating each file in separate thread, which might be slower because of synchronization between threads.
Therefore I kept the property change event for all of the files and changed the 2 listeners to run in background, updating all the files with a single background thread.
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
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)