mbien commented on code in PR #9324:
URL: https://github.com/apache/netbeans/pull/9324#discussion_r3036511905
##########
ide/git/src/org/netbeans/modules/git/GitVCS.java:
##########
@@ -111,6 +113,13 @@ public void propertyChange(PropertyChangeEvent event) {
if
(event.getPropertyName().equals(FileStatusCache.PROP_FILE_STATUS_CHANGED)) {
FileStatusCache.ChangedEvent changedEvent =
(FileStatusCache.ChangedEvent) event.getNewValue();
fireStatusChanged(changedEvent.getFile());
+ } else if
(event.getPropertyName().equals(FileStatusCache.PROP_FILES_STATUS_CHANGED)) {
+ List<FileStatusCache.ChangedEvent> changedEvents =
(List<FileStatusCache.ChangedEvent>) event.getNewValue();
+ Set<File> files = new HashSet<>(changedEvents.size());
Review Comment:
this is super nitpicky. but the constructor arg isn't for the element count,
it is to size the internal table - which sometimes leads to initial sizes which
are too small.
if you want you can bump the module to `javac.release=21` (see
project.properties) and then use the `HashSet.newHashSet(numElements)` factory
which does some math before setting the size of the internal table.
feel free to update the commit and force push in place.
##########
ide/git/src/org/netbeans/modules/git/FileStatusCache.java:
##########
@@ -579,8 +585,8 @@ private void refreshStatusesBatch (Map<File, GitStatus>
interestingFiles) {
}
updateIndexBatch(indexUpdates);
}
- for (ChangedEvent event : events) {
- fireFileStatusChanged(event);
+ if (!events.isEmpty()) {
+ listenerSupport.firePropertyChange(PROP_FILES_STATUS_CHANGED,
null, events);
Review Comment:
good catch. aggregating events can certainly reduce overhead under load.
Reminds me a little on https://github.com/apache/netbeans/pull/8955 where I saw
1.5mil event handler invocations when a large project group opened.
##########
ide/git/src/org/netbeans/modules/git/ui/diff/MultiDiffPanelController.java:
##########
@@ -1123,15 +1123,24 @@ public void propertyChange (PropertyChangeEvent evt) {
if
(FileStatusCache.PROP_FILE_STATUS_CHANGED.equals(evt.getPropertyName())) {
FileStatusCache.ChangedEvent changedEvent =
(FileStatusCache.ChangedEvent) evt.getNewValue();
if (LOG.isLoggable(Level.FINE)) {
- LOG.log(Level.FINE, "File status for file {0} changed from {1}
to {2}", new Object[] {
- changedEvent.getFile(),
+ LOG.log(Level.FINE, "File status for file {0} changed from {1}
to {2}", new Object[] {
+ changedEvent.getFile(),
changedEvent.getOldInfo(),
changedEvent.getNewInfo() } );
}
- if (revisionLeft == Revision.HEAD // remove when we're able to
refresh single file changes for Local vs. any revision
+ if (revisionLeft == Revision.HEAD // remove when we're able to
refresh single file changes for Local vs. any revision
&& revisionRight == Revision.LOCAL &&
affectsView(changedEvent)) {
applyChange(changedEvent);
}
+ } else if
(FileStatusCache.PROP_FILES_STATUS_CHANGED.equals(evt.getPropertyName())) {
+ @SuppressWarnings("unchecked")
+ List<FileStatusCache.ChangedEvent> changedEvents =
(List<FileStatusCache.ChangedEvent>) evt.getNewValue();
+ for (FileStatusCache.ChangedEvent changedEvent : changedEvents) {
+ if (revisionLeft == Revision.HEAD // remove when we're able to
refresh single file changes for Local vs. any revision
+ && revisionRight == Revision.LOCAL &&
affectsView(changedEvent)) {
Review Comment:
could the `revisionLeft/Right` check be moved before the loop?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
